diff --git a/Server/CLAUDE.md b/Server/CLAUDE.md index e7515ed6..8659323e 100644 --- a/Server/CLAUDE.md +++ b/Server/CLAUDE.md @@ -11,7 +11,9 @@ prometheus. - `db/` hand-written query wrappers; `db/dbgen/` is generated (see `db-change`) - `cmd/` executable tooling, one `package main` per subdirectory — `cmd/genprotocol/` regenerates the protocol constants from `protocol/schema.json`, - `cmd/seed/` fills a dev database (`go run ./cmd/seed -confirm-dev`). + `cmd/seed/` fills a dev database (`go run ./cmd/seed -confirm-dev`), + `cmd/dbinventory/` prints the `db`-importer table for + `docs/architecture/server-boundaries.md` (exits 1 on an unlisted importer). `scripts/` holds shell/JS tooling only; no Go entry point lives there - `admin/` web admin panel · `updater/` self-update + signature verification · `plugin/` WASM plugin runtime (`-tags wazero`) · `telemetry/` OTel (`-tags otel`) @@ -34,3 +36,7 @@ prometheus. - Prefer the standard library. `syncutil` exists so lock usage is uniform and detectable; do not hand-roll around it. `Server/invariants/` enforces this at `go test` time; exceptions are greppable via `grep -rn "invariant:allow" Server/`. +- Only `db/` and `service/` import `db` freely. Any other production file that + imports it needs a row in `invariants/db_import_boundary.go` (`DBImportAllow`) + with a disposition and reason — the B3 inventory, which only shrinks. New + persistence goes behind a service, not into a handler. diff --git a/Server/cmd/dbinventory/main.go b/Server/cmd/dbinventory/main.go new file mode 100644 index 00000000..7ba31b7f --- /dev/null +++ b/Server/cmd/dbinventory/main.go @@ -0,0 +1,458 @@ +// Command dbinventory lists every production Go file above the domain layer +// that imports the db package, and what it uses it for: db.* types, db.* +// package functions and sentinels, and method calls on a *db.DB value. +// +// It is the measurement behind docs/architecture/server-boundaries.md (B3-0) +// and prints a Markdown table so the document can be regenerated: +// +// cd Server && go run ./cmd/dbinventory +// +// The analysis is syntactic (go/parser + go/ast, no type information), like +// Server/invariants: a *db.DB method call is recognised when the receiver is +// an identifier declared with type *db.DB in the same file (parameter, result, +// var, or a name assigned from db.Open*), or a selector whose final field is +// declared *db.DB anywhere in the same package (h.db.X, s.deps.DB.X). That +// covers every shape in the tree today; a new shape shows up as a file with +// an import and no recorded use, which is itself a row worth reading. +package main + +import ( + "flag" + "fmt" + "go/ast" + "go/parser" + "go/token" + "io/fs" + "maps" + "os" + "path" + "path/filepath" + "sort" + "strings" + + "github.com/J3vb/OwnCord/Server/invariants" +) + +const dbImportPath = "github.com/J3vb/OwnCord/Server/db" + +// layerDirs are the top-level packages that may import db freely and are +// therefore not inventoried. Matched on the root-relative path, so a nested +// directory that happens to share a name (api/service/) is still inventoried +// — the same rule db-import-boundary applies. +var layerDirs = map[string]bool{"db": true, "service": true} + +// skipNames are never code at any depth: fixtures and vendored JS. +var skipNames = map[string]bool{"testdata": true, "node_modules": true} + +type kind int + +const ( + kindType kind = iota + kindFunc + kindValue // var or const, e.g. sentinel errors +) + +type fileUse struct { + rel string + types map[string]int + funcs map[string]int + values map[string]int + methods map[string]int +} + +func main() { + root := flag.String("root", ".", "Server module root") + flag.Parse() + + fset := token.NewFileSet() + dbKinds, err := declKinds(fset, filepath.Join(*root, "db")) + if err != nil { + fmt.Fprintln(os.Stderr, err) + os.Exit(1) + } + + files, err := productionFiles(*root) + if err != nil { + fmt.Fprintln(os.Stderr, err) + os.Exit(1) + } + + // Pass 1: parse everything, collect struct fields typed *db.DB per package. + parsed := map[string]*ast.File{} + fieldsByPkg := map[string]map[string]bool{} + for _, rel := range files { + f, err := parser.ParseFile(fset, filepath.Join(*root, rel), nil, 0) + if err != nil { + fmt.Fprintln(os.Stderr, err) + os.Exit(1) + } + parsed[rel] = f + alias := dbAlias(f) + if alias == "" { + continue + } + dir := path.Dir(rel) + if fieldsByPkg[dir] == nil { + fieldsByPkg[dir] = map[string]bool{} + } + for name := range dbDBFields(f, alias) { + fieldsByPkg[dir][name] = true + } + } + + // Pass 2: per-file uses. + var rows []fileUse + for _, rel := range files { + f := parsed[rel] + alias := dbAlias(f) + if alias == "" { + continue + } + rows = append(rows, analyze(f, rel, alias, dbKinds, fieldsByPkg[path.Dir(rel)])) + } + sort.Slice(rows, func(i, j int) bool { return rows[i].rel < rows[j].rel }) + printTable(rows) +} + +// productionFiles returns slash-separated .go paths under root, excluding +// tests and skipDirs, sorted. +func productionFiles(root string) ([]string, error) { + var out []string + err := filepath.WalkDir(root, func(p string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + if d.IsDir() { + if p == root { + return nil + } + rel, err := filepath.Rel(root, p) + if err != nil { + return err + } + if layerDirs[filepath.ToSlash(rel)] || skipNames[d.Name()] || strings.HasPrefix(d.Name(), ".") { + return fs.SkipDir + } + return nil + } + if !strings.HasSuffix(p, ".go") || strings.HasSuffix(p, "_test.go") { + return nil + } + rel, err := filepath.Rel(root, p) + if err != nil { + return err + } + out = append(out, filepath.ToSlash(rel)) + return nil + }) + sort.Strings(out) + return out, err +} + +// declKinds parses the db package's production files and maps every exported +// top-level name to its kind, so a db.X selector can be classified exactly. +func declKinds(fset *token.FileSet, dir string) (map[string]kind, error) { + entries, err := os.ReadDir(dir) + if err != nil { + return nil, fmt.Errorf("reading %s: %w", dir, err) + } + kinds := map[string]kind{} + for _, e := range entries { + name := e.Name() + if e.IsDir() || !strings.HasSuffix(name, ".go") || strings.HasSuffix(name, "_test.go") { + continue + } + f, err := parser.ParseFile(fset, filepath.Join(dir, name), nil, 0) + if err != nil { + return nil, fmt.Errorf("parsing %s: %w", name, err) + } + for _, decl := range f.Decls { + switch d := decl.(type) { + case *ast.FuncDecl: + if d.Recv == nil && d.Name.IsExported() { + kinds[d.Name.Name] = kindFunc + } + case *ast.GenDecl: + for _, spec := range d.Specs { + switch s := spec.(type) { + case *ast.TypeSpec: + if s.Name.IsExported() { + kinds[s.Name.Name] = kindType + } + case *ast.ValueSpec: + for _, n := range s.Names { + if n.IsExported() { + kinds[n.Name] = kindValue + } + } + } + } + } + } + } + return kinds, nil +} + +// dbAlias returns the local name the file imports the db package under, or +// "" if it does not import it. +func dbAlias(f *ast.File) string { + for _, imp := range f.Imports { + if strings.Trim(imp.Path.Value, `"`) != dbImportPath { + continue + } + if imp.Name != nil { + return imp.Name.Name + } + return "db" + } + return "" +} + +// isDBPtr reports whether expr is *.DB. +func isDBPtr(expr ast.Expr, alias string) bool { + star, ok := expr.(*ast.StarExpr) + if !ok { + return false + } + sel, ok := star.X.(*ast.SelectorExpr) + if !ok { + return false + } + x, ok := sel.X.(*ast.Ident) + return ok && x.Name == alias && sel.Sel.Name == "DB" +} + +// dbDBFields returns the names of struct fields typed *db.DB in the file. +func dbDBFields(f *ast.File, alias string) map[string]bool { + out := map[string]bool{} + ast.Inspect(f, func(n ast.Node) bool { + st, ok := n.(*ast.StructType) + if !ok { + return true + } + for _, fld := range st.Fields.List { + if isDBPtr(fld.Type, alias) { + for _, name := range fld.Names { + out[name.Name] = true + } + } + } + return true + }) + return out +} + +func analyze(f *ast.File, rel, alias string, dbKinds map[string]kind, dbFields map[string]bool) fileUse { + u := fileUse{rel: rel, types: map[string]int{}, funcs: map[string]int{}, values: map[string]int{}, methods: map[string]int{}} + dbVars := collectDBVars(f, alias) + countMethodCalls(f, dbVars, dbFields, u.methods) + classifySelectors(f, alias, dbKinds, &u) + return u +} + +// pkgSelector returns the selector's field name when expr is .X. +func pkgSelector(expr ast.Expr, alias string) (string, bool) { + sel, ok := expr.(*ast.SelectorExpr) + if !ok { + return "", false + } + pkg, ok := sel.X.(*ast.Ident) + if !ok || pkg.Name != alias { + return "", false + } + return sel.Sel.Name, true +} + +// collectDBVars returns identifiers declared with type *db.DB: params, +// results, struct fields, vars, and names assigned from a db.Open* call. +func collectDBVars(f *ast.File, alias string) map[string]bool { + dbVars := map[string]bool{} + add := func(names []*ast.Ident) { + for _, n := range names { + dbVars[n.Name] = true + } + } + ast.Inspect(f, func(n ast.Node) bool { + switch x := n.(type) { + case *ast.Field: + if isDBPtr(x.Type, alias) { + add(x.Names) + } + case *ast.ValueSpec: + if x.Type != nil && isDBPtr(x.Type, alias) { + add(x.Names) + } + case *ast.AssignStmt: + for i, rhs := range x.Rhs { + if openAssign(rhs, alias) && i < len(x.Lhs) { + if id, ok := x.Lhs[i].(*ast.Ident); ok { + dbVars[id.Name] = true + } + } + } + } + return true + }) + return dbVars +} + +// openAssign reports whether expr is a call to .Open*(...). +func openAssign(expr ast.Expr, alias string) bool { + call, ok := expr.(*ast.CallExpr) + if !ok { + return false + } + name, ok := pkgSelector(call.Fun, alias) + return ok && strings.HasPrefix(name, "Open") +} + +// countMethodCalls tallies calls whose receiver is a *db.DB identifier or a +// selector ending in a *db.DB struct field. +func countMethodCalls(f *ast.File, dbVars, dbFields map[string]bool, methods map[string]int) { + ast.Inspect(f, func(n ast.Node) bool { + call, ok := n.(*ast.CallExpr) + if !ok { + return true + } + sel, ok := call.Fun.(*ast.SelectorExpr) + if !ok { + return true + } + switch x := sel.X.(type) { + case *ast.Ident: + if dbVars[x.Name] { + methods[sel.Sel.Name]++ + } + case *ast.SelectorExpr: + if dbFields[x.Sel.Name] { + methods[sel.Sel.Name]++ + } + } + return true + }) +} + +// classifySelectors buckets every .X selector by what db declares X as. +func classifySelectors(f *ast.File, alias string, dbKinds map[string]kind, u *fileUse) { + ast.Inspect(f, func(n ast.Node) bool { + expr, ok := n.(ast.Expr) + if !ok { + return true + } + name, ok := pkgSelector(expr, alias) + if !ok { + return true + } + switch k, known := dbKinds[name]; { + case !known: + u.values["?"+name]++ + case k == kindType: + u.types[name]++ + case k == kindFunc: + u.funcs[name]++ + default: + u.values[name]++ + } + return true + }) +} + +func joined(m map[string]int) string { + if len(m) == 0 { + return "—" + } + keys := make([]string, 0, len(m)) + for k := range m { + keys = append(keys, k) + } + sort.Strings(keys) + parts := make([]string, 0, len(keys)) + for _, k := range keys { + if m[k] > 1 { + parts = append(parts, fmt.Sprintf("%s×%d", k, m[k])) + } else { + parts = append(parts, k) + } + } + return "`" + strings.Join(parts, "` `") + "`" +} + +func sum(m map[string]int) int { + n := 0 + for _, v := range m { + n += v + } + return n +} + +func printTable(rows []fileUse) { + byPkg := map[string]int{} + byDisposition := map[string]int{} + byFamily := map[string]int{} + typeOnly, unlisted := 0, 0 + fmt.Println("| File | `db.*` types | `db.*` funcs and sentinels | `*db.DB` method calls | Shape | Disposition | Family | Why |") + fmt.Println("| --- | --- | --- | --- | --- | --- | --- | --- |") + for _, r := range rows { + byPkg[path.Dir(r.rel)]++ + shape := "calls" + if sum(r.funcs)+sum(r.values)+sum(r.methods) == 0 { + shape = "type-only" + typeOnly++ + } + entry, listed := invariants.DBImportAllow[r.rel] + if !listed { + unlisted++ + entry = invariants.DBImportEntry{Disposition: "**UNLISTED**", Note: "fails db-import-boundary"} + } + byDisposition[entry.Disposition]++ + if entry.Family != "" { + byFamily[entry.Family]++ + } + family := entry.Family + if family == "" { + family = "—" + } + fmt.Printf("| `%s` | %s | %s | %s | %s | %s | %s | %s |\n", + r.rel, joined(r.types), mergeFV(r), joined(r.methods), shape, entry.Disposition, family, entry.Note) + } + fmt.Printf("\n%d files import `db` outside `db/` and `service/` (%s); %d are type-only; %d unlisted.\n", + len(rows), countList(byPkg), typeOnly, unlisted) + fmt.Printf("Dispositions: %s. Move targets: %s.\n", countList(byDisposition), countList(byFamily)) + stale := 0 + present := map[string]bool{} + for _, r := range rows { + present[r.rel] = true + } + for rel := range invariants.DBImportAllow { + if !present[rel] { + stale++ + fmt.Printf("STALE allowlist row (file no longer imports db): `%s`\n", rel) + } + } + if unlisted > 0 || stale > 0 { + os.Exit(1) + } +} + +// countList renders a count map as "a 1, b 2", keys sorted. +func countList(m map[string]int) string { + keys := make([]string, 0, len(m)) + for k := range m { + keys = append(keys, k) + } + sort.Strings(keys) + parts := make([]string, 0, len(keys)) + for _, k := range keys { + parts = append(parts, fmt.Sprintf("%s %d", k, m[k])) + } + return strings.Join(parts, ", ") +} + +func mergeFV(r fileUse) string { + m := make(map[string]int, len(r.funcs)+len(r.values)) + for k, v := range r.funcs { + m[k+"()"] = v + } + maps.Copy(m, r.values) + return joined(m) +} diff --git a/Server/cmd/dbinventory/main_test.go b/Server/cmd/dbinventory/main_test.go new file mode 100644 index 00000000..7802b273 --- /dev/null +++ b/Server/cmd/dbinventory/main_test.go @@ -0,0 +1,45 @@ +package main + +import ( + "os" + "path/filepath" + "reflect" + "testing" +) + +// TestProductionFilesExemptsOnlyTopLevelLayers pins the walker's exemption to +// the root-relative path: Server/db and Server/service are the layers that +// may import db; a nested directory that shares a name (api/service/) is +// production code above the domain layer and must be inventoried. Tests, +// testdata and hidden directories are skipped at any depth. +func TestProductionFilesExemptsOnlyTopLevelLayers(t *testing.T) { + root := t.TempDir() + for _, p := range []string{ + "api/w.go", + "api/w_test.go", + "api/service/x.go", + "api/db/y.go", + "service/y.go", + "db/z.go", + "db/dbgen/q.go", + "ws/testdata/fixture.go", + ".hidden/h.go", + "main.go", + } { + full := filepath.Join(root, filepath.FromSlash(p)) + if err := os.MkdirAll(filepath.Dir(full), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(full, []byte("package x\n"), 0o644); err != nil { + t.Fatal(err) + } + } + got, err := productionFiles(root) + if err != nil { + t.Fatal(err) + } + want := []string{"api/db/y.go", "api/service/x.go", "api/w.go", "main.go"} + if !reflect.DeepEqual(got, want) { + t.Fatalf("productionFiles = %v, want %v", got, want) + } +} diff --git a/Server/invariants/db_import_boundary.go b/Server/invariants/db_import_boundary.go new file mode 100644 index 00000000..d9650b4c --- /dev/null +++ b/Server/invariants/db_import_boundary.go @@ -0,0 +1,130 @@ +package invariants + +import ( + "go/ast" + "go/token" + "strings" +) + +// dbImportBoundaryID is the rule's stable id (a const for the same +// initialization-cycle reason as syncutilLocksID). +const dbImportBoundaryID = "db-import-boundary" + +// dbImportPath is the persistence package every rule here is about. +const dbImportPath = "github.com/J3vb/OwnCord/Server/db" + +// DBImportEntry is one row of the B3-0 boundary inventory: why a production +// file above the domain layer is allowed to import db, and where B3-8 sends +// it. Dispositions are the layout-refactor supplement's four: +// +// - move: persistence or domain decisions that belong behind a service; +// Family names the service that takes them. +// - adapter: a transport adapter that uses db types or pure helpers only +// (response shapes, status helpers) — no persistence calls. +// - boundary: an explicit composition or transaction boundary (the process +// entry, a CLI, health probing) that legitimately owns a handle. +// - remove: the import is unnecessary and goes. +// +// docs/architecture/server-boundaries.md is generated from this map by +// `go run ./cmd/dbinventory`; edit here, then regenerate. +type DBImportEntry struct { + Disposition string + Family string + Note string +} + +// DBImportAllow is the inventory. A production file outside db/ and service/ +// that imports db and is not listed here fails db-import-boundary; a listed +// file that stops importing db fails TestDBImportAllowIsLive. B3-2 and B3-8 +// delete rows as families move — the list only shrinks. +var DBImportAllow = map[string]DBImportEntry{ + // ── admin ───────────────────────────────────────────────────────────── + "admin/admin.go": {"boundary", "", "holds the handle for the admin mux; no calls"}, + "admin/api.go": {"boundary", "", "passes the handle to handlers; no calls"}, + "admin/backup_maintenance.go": {"move", "settings-ops", "BackupToSafe, integrity check, settings reads"}, + "admin/handlers_backup.go": {"move", "settings-ops", "backup trigger and download; raw SQLDb for VACUUM INTO"}, + "admin/handlers_channel_perms.go": {"move", "channel", "override CRUD decides permission policy in the handler"}, + "admin/handlers_channels.go": {"move", "channel", "channel CRUD + audit"}, + "admin/handlers_roles.go": {"move", "role", "two reads; service/role.go already owns the writes"}, + "admin/handlers_settings.go": {"move", "settings-ops", "BeginTx in a handler; TOTP census"}, + "admin/handlers_tokens.go": {"move", "auth", "API-token CRUD duplicated in token_cli.go"}, + "admin/handlers_users.go": {"move", "user", "user list, stats, lookups"}, + "admin/helpers.go": {"adapter", "", "Role/User types in response helpers"}, + "admin/logstream.go": {"boundary", "", "handle threaded to the SSE stream's auth check; no calls"}, + "admin/middleware.go": {"move", "auth", "owner gate re-reads the role — OC-0345"}, + "admin/setup_handler.go": {"move", "auth", "first-run owner creation (setup sub-family)"}, + "admin/setup_wizard.go": {"move", "auth", "BeginTx for the wizard; setup sub-family"}, + "admin/types.go": {"adapter", "", "response DTOs; the one GetRoleByID moves with handlers_users"}, + // ── api ─────────────────────────────────────────────────────────────── + "api/auth_handler.go": {"move", "auth", "B3-2 slice: register/login/logout/delete own the DB"}, + "api/channel_handler.go": {"adapter", "", "response types only; service owns the calls"}, + "api/dm_handler.go": {"adapter", "", "DM response types + pure status helpers"}, + "api/emoji_handler.go": {"adapter", "", "Emoji/User types only"}, + "api/gif_handler.go": {"adapter", "", "handle in the signature, unused for calls"}, + "api/invite_handler.go": {"adapter", "", "Invite/User types only"}, + "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/totp_handler.go": {"move", "auth", "B3-2 slice: TOTP enrol/verify write the user row"}, + "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"}, + "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"}, + "ws/event.go": {"adapter", "", "pure BroadcastStatus helper"}, + "ws/event_persister.go": {"adapter", "", "PersistedEvent type; store is an interface"}, + "ws/eventstore.go": {"adapter", "", "PersistedEvent type; store is an interface"}, + "ws/handlers.go": {"move", "channel", "channel, role, session-ban and DM reads in command handlers"}, + "ws/handlers_chat.go": {"adapter", "", "pure NewDMChannelInfo helper"}, + "ws/hub.go": {"move", "settings-ops", "GetSetting at construction"}, + "ws/hub_broadcast.go": {"move", "channel", "visibility refresh reads channels, roles, users"}, + "ws/hub_sweep.go": {"move", "voice", "stale-voice sweep reads and leaves"}, + "ws/messages.go": {"adapter", "", "wire types + pure status helpers"}, + "ws/serve.go": {"move", "connection", "connect/disconnect lifecycle; B3-5 splits it by family first"}, + "ws/serve_auth.go": {"move", "auth", "session lookup on the WebSocket handshake"}, + "ws/serve_pumps.go": {"move", "user", "MarkUserDisconnected on pump exit"}, + "ws/serve_ready.go": {"move", "channel", "ready snapshot: channels, overrides, unreads, DMs, members"}, + "ws/voice_join.go": {"move", "voice", "voice state reads and writes"}, + "ws/voice_moderation.go": {"move", "voice", "mute/deafen/move persist voice state"}, +} + +// dbImportBoundary fails on any production file above the domain layer that +// imports db without an inventory row. db/ and service/ are the layers that +// may import it; everything else must be in DBImportAllow, which is the B3-0 +// inventory (docs/architecture/server-boundaries.md is generated from it). +var dbImportBoundary = Rule{ + ID: dbImportBoundaryID, + Scope: nil, // every directory; the layers that may import are excluded in Check + Check: checkDBImportBoundary, +} + +func checkDBImportBoundary(f *ast.File, fset *token.FileSet, rel string) []Violation { + if strings.HasPrefix(rel, "db/") || strings.HasPrefix(rel, "service/") { + return nil + } + if _, listed := DBImportAllow[rel]; listed { + return nil + } + for _, imp := range f.Imports { + if strings.Trim(imp.Path.Value, `"`) != dbImportPath { + continue + } + return []Violation{{ + Rule: dbImportBoundaryID, + File: rel, + Line: fset.Position(imp.Pos()).Line, + Msg: "imports Server/db above the domain layer without an inventory row; " + + "route the call through a service (see docs/architecture/server-boundaries.md), " + + "or add a DBImportAllow entry with a disposition and reason", + }} + } + return nil +} diff --git a/Server/invariants/db_import_boundary_test.go b/Server/invariants/db_import_boundary_test.go new file mode 100644 index 00000000..71a1f837 --- /dev/null +++ b/Server/invariants/db_import_boundary_test.go @@ -0,0 +1,112 @@ +package invariants + +import ( + "go/token" + "os" + "path/filepath" + "strings" + "testing" +) + +func TestDBImportBoundary(t *testing.T) { + const importDB = `import "github.com/J3vb/OwnCord/Server/db"` + tests := []struct { + name string + path string + src string + want int + }{ + { + name: "unlisted api file importing db is flagged", + path: "api/brand_new_handler.go", + src: "package api\n" + importDB + "\nvar _ *db.DB\n", + want: 1, + }, + { + name: "aliased import is still flagged", + path: "ws/brand_new.go", + src: "package ws\nimport store \"github.com/J3vb/OwnCord/Server/db\"\nvar _ *store.DB\n", + want: 1, + }, + { + name: "listed file is allowed", + path: "api/auth_handler.go", + src: "package api\n" + importDB + "\nvar _ *db.DB\n", + want: 0, + }, + { + name: "service may import db", + path: "service/anything.go", + src: "package service\n" + importDB + "\nvar _ *db.DB\n", + want: 0, + }, + { + name: "db itself is out of scope", + path: "db/anything.go", + src: "package db\n" + importDB + "\n", + want: 0, + }, + { + name: "unlisted file without the import is clean", + path: "api/brand_new_handler.go", + src: "package api\nimport \"net/http\"\nvar _ http.Handler\n", + want: 0, + }, + { + name: "a sibling module path is not the db package", + path: "api/brand_new_handler.go", + src: "package api\nimport \"github.com/J3vb/OwnCord/Server/db/dbgen\"\nvar _ dbgen.Queries\n", + want: 0, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + fset := token.NewFileSet() + got := checkSourceWith([]Rule{dbImportBoundary}, fset, tt.path, []byte(tt.src)) + if len(got) != tt.want { + t.Fatalf("want %d violation(s), got %d: %v", tt.want, len(got), got) + } + for _, v := range got { + if v.Rule != dbImportBoundaryID { + t.Errorf("rule id = %q, want %q", v.Rule, dbImportBoundaryID) + } + if !strings.Contains(v.Msg, "server-boundaries.md") { + t.Errorf("message must point at the inventory document: %q", v.Msg) + } + } + }) + } +} + +// TestDBImportAllowIsLive keeps the inventory honest in the other direction: +// every allowlisted path must exist and still import db. A row for a file +// that moved behind a service (or was renamed) is stale and must be deleted — +// the list only shrinks. +func TestDBImportAllowIsLive(t *testing.T) { + for rel, entry := range DBImportAllow { + p := filepath.Join("..", filepath.FromSlash(rel)) + src, err := os.ReadFile(p) + if err != nil { + t.Errorf("DBImportAllow[%q]: %v — delete the row", rel, err) + continue + } + if !strings.Contains(string(src), `"`+dbImportPath+`"`) { + t.Errorf("DBImportAllow[%q] no longer imports db — delete the row", rel) + } + switch entry.Disposition { + case "move": + if entry.Family == "" { + t.Errorf("DBImportAllow[%q]: a move needs a target family", rel) + } + case "adapter", "boundary", "remove": + if entry.Family != "" { + t.Errorf("DBImportAllow[%q]: %s rows carry no family", rel, entry.Disposition) + } + default: + t.Errorf("DBImportAllow[%q]: unknown disposition %q", rel, entry.Disposition) + } + if entry.Note == "" { + t.Errorf("DBImportAllow[%q]: the reason is mandatory", rel) + } + } +} diff --git a/Server/invariants/invariants.go b/Server/invariants/invariants.go index 04e279f6..64b94614 100644 --- a/Server/invariants/invariants.go +++ b/Server/invariants/invariants.go @@ -61,7 +61,7 @@ func (r Rule) inScope(dir string) bool { } // Rules is the registry every gate runs. -var Rules = []Rule{syncutilLocks} +var Rules = []Rule{syncutilLocks, dbImportBoundary} // allowPrefix introduces a line-scoped suppression: // diff --git a/docs/architecture/README.md b/docs/architecture/README.md index a777670e..78fed0ab 100644 --- a/docs/architecture/README.md +++ b/docs/architecture/README.md @@ -11,17 +11,18 @@ natively) followed by a prose explanation and a **Source of truth** file list. ## Index -| Doc | Diagrams | Covers | -| ---------------------------------------------- | ------------------------------------------------ | -------------------------------------------------------------------------------------------------------------------------------------- | -| [system-overview.md](system-overview.md) | D1 System context, D8 Deployment topology | All processes, trust boundaries, ports, single-instance constraints | -| [server.md](server.md) | D2 Server package map, D3 REST request lifecycle | Go package structure, DB-access styles, middleware chain | -| [websocket.md](websocket.md) | D4 WS connect / replay / dispatch | Real-time engine: auth handshake, 3-tier reconnect replay, backpressure, typed dispatch | -| [data-model.md](data-model.md) | D5 Entity-relationship overview | All 26 tables from migrations 001–028, grouped by domain | -| [voice-e2ee.md](voice-e2ee.md) | D6 Voice + E2EE flow | LiveKit token flow, loopback TLS tunnel, ECDH key-holder relay | -| [client.md](client.md) | D7 Client module map | Tauri client: bootstrap, dispatcher, stores, Rust sidecars (structure, as-built) | -| [ux/](ux/README.md) | UX flow + state diagrams | Client **behavior** spec (target state): what every view does and how it reacts to events, permissions, and failure | -| [platform-contracts.md](platform-contracts.md) | — | Desktop/browser **seam** (target state): where native dependencies will be isolated, and the three that have no browser equivalent | -| [plugins.md](plugins.md) | — | Experimental WASM plugin boundary: off twice and compiled out of releases, no API promise, post-beta candidates, core that never moves | +| Doc | Diagrams | Covers | +| ---------------------------------------------- | ------------------------------------------------ | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| [system-overview.md](system-overview.md) | D1 System context, D8 Deployment topology | All processes, trust boundaries, ports, single-instance constraints | +| [server.md](server.md) | D2 Server package map, D3 REST request lifecycle | Go package structure, DB-access styles, middleware chain | +| [websocket.md](websocket.md) | D4 WS connect / replay / dispatch | Real-time engine: auth handshake, 3-tier reconnect replay, backpressure, typed dispatch | +| [data-model.md](data-model.md) | D5 Entity-relationship overview | All 26 tables from migrations 001–028, grouped by domain | +| [voice-e2ee.md](voice-e2ee.md) | D6 Voice + E2EE flow | LiveKit token flow, loopback TLS tunnel, ECDH key-holder relay | +| [client.md](client.md) | D7 Client module map | Tauri client: bootstrap, dispatcher, stores, Rust sidecars (structure, as-built) | +| [ux/](ux/README.md) | UX flow + state diagrams | Client **behavior** spec (target state): what every view does and how it reacts to events, permissions, and failure | +| [platform-contracts.md](platform-contracts.md) | — | Desktop/browser **seam** (target state): where native dependencies will be isolated, and the three that have no browser equivalent | +| [server-boundaries.md](server-boundaries.md) | — | B3-0 inventory: every file above the domain layer that imports `db`, with a disposition and target family; hub setters, locks and the start/stop defer stack; the auth slice's before-graph. Generated table, enforced by `db-import-boundary` | +| [plugins.md](plugins.md) | — | Experimental WASM plugin boundary: off twice and compiled out of releases, no API promise, post-beta candidates, core that never moves | ### Structure vs. behavior diff --git a/docs/architecture/server-boundaries.md b/docs/architecture/server-boundaries.md new file mode 100644 index 00000000..56f1a743 --- /dev/null +++ b/docs/architecture/server-boundaries.md @@ -0,0 +1,236 @@ +# Server boundaries — database-call and lifecycle inventory + +**Written:** 2026-08-29 (B3-0), measured at `dev` `ad4defc2`. +**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 +paste its output between the markers below. The tool exits non-zero when a +file imports `db` without a row, or a row names a file that no longer imports +it — the same two failures `go test ./invariants/` reports. + +This is the inventory the roadmap's B3 entry gate asks for ("hotspots and +direct database call sites have an owned inventory") and the evidence its exit +gate consumes ("every direct database use above the domain layer is justified +or removed"). It answers three questions for every production Go file above +the domain layer: does it import `db`; what does it use `db` for; and what +happens to that use — one of four dispositions from the +[layout-refactor supplement](../plans/developer-experience-layout-refactor-2026-08-29.md): + +| Disposition | Meaning | Rows | +| ----------- | --------------------------------------------------------------------------------------------------------------------- | ---: | +| `move` | persistence or a domain decision that belongs behind a service; **Family** names the service B3-8 (or B3-2) builds | 28 | +| `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 | +| `remove` | the import is unnecessary and goes | 0 | + +The rows live in code, not only here: `Server/invariants/db_import_boundary.go` +holds them as `DBImportAllow`, the `db-import-boundary` rule fails any new +importer that has no row, and `TestDBImportAllowIsLive` fails any row whose +file stopped importing `db`. The list only shrinks — B3-2 deletes the two auth +handler rows, B3-8 deletes a family's rows as it moves. + +## How the measurement works + +`Server/cmd/dbinventory` is syntactic (`go/parser` + `go/ast`, no type +information), like the invariants package. It records three things per file: + +- **`db.*` types** — selectors that `db` declares as types (`db.User`, + `db.Channel`, …). A file whose only use is types is **type-only**: it + shapes data, it does not persist. +- **`db.*` funcs and sentinels** — package-level functions (`db.WriteAudit()`, + `db.Open()`) and values (`db.ErrNotFound`). Pure helpers such as + `db.StatusForViewer()` and `db.BroadcastStatus()` land here too; they are + computations over already-loaded rows, not queries. +- **`*db.DB` method calls** — calls whose receiver is an identifier declared + `*db.DB` in the file (parameter, result, var, or assigned from `db.Open*`), + or a selector whose final field is declared `*db.DB` anywhere in the same + package (`h.db.X`, `s.deps.DB.X`). This is the persistence surface the + dispositions are about. + +A shape the walker cannot see (a `*db.DB` reaching a file through an untyped +interface, say) would show up as a row with an import and nothing recorded — +which is a row worth reading, and none exists today. + +## Database-call inventory + + + +| 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/auth_handler.go` | `DB×11` `Session` `User×5` | `ErrLastAdmin` `ErrNotFound×2` `IsUniqueConstraintError()` `WriteAudit()×5` | `CreateSession×2` `CreateUserWithInvite` `DeleteAccount` `DeleteSession` `GetSetting` `GetUserByID` `GetUserByUsername` `UpdateUserCustomStatus` | calls | move | auth | B3-2 slice: register/login/logout/delete own the DB | +| `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` | — | `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×6` | — | `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/totp_handler.go` | `DB×5` `Session×2` `User×4` | `WriteAudit()×3` | `DeleteOtherSessions×2` `GetUserByID` `UpdateUserTOTPSecret×2` | calls | move | auth | B3-2 slice: TOTP enrol/verify write the user row | +| `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/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 | + +51 files import `db` outside `db/` and `service/` (. 2, admin 16, api 12, auth 2, cmd/seed 1, plugin 1, ws 17); 14 are type-only; 0 unlisted. +Dispositions: adapter 17, boundary 6, move 28. Move targets: auth 9, channel 6, connection 1, role 1, settings-ops 4, upload 2, user 2, voice 3. + + + +Reading the table: + +- **Type-only files (14)** need no service; they stay `adapter`. The `db` + types they use are the wire and response shapes. Whether those types should + live outside `db` is a B3-8 question per family, not a boundary violation. +- **`ws/serve_ready.go`** (45 references, 7 distinct queries) is the single + heaviest reader: the ready snapshot reads channels, overrides, unreads, DM + channels, members, roles and voice states in one place. B3-5 keeps it as + the "fresh-connect initialisation" file and B3-8's channel family gives it + a snapshot service. +- **Two raw SQL escapes** exist above the domain layer: + `api/upload_handler.go` (`QueryRowContext`) and `admin/handlers_backup.go` + (`SQLDb` for `VACUUM INTO`). Both are `move`; the backup one may end as an + explicit `boundary` once `settings-ops` owns backups — the row is decided + when that family moves, not now. +- **Duplicated persistence** is visible in the families: API-token CRUD in + `admin/handlers_tokens.go` and `token_cli.go`; owner-role reads in + `admin/middleware.go` (OC-0345) and `api/middleware.go`; voice state in + `ws/hub_sweep.go`, `voice_join.go`, `voice_moderation.go`. One service per + family removes each duplicate. +- **`ws/serve.go`** is the one `connection` row: it is not a domain family but + the connect/disconnect lifecycle that touches four of them. B3-5 splits it + by responsibility first; the pieces then join their families' rows. + +## Hub lifecycle inventory + +Input to B3-3 (`internal/app/`) and B3-4 (constructor options). Measured at +the same commit. + +### Construction and setters (S-11) + +`ws.NewHub(database, limiter, svc)` is called **once**, inside +`api.NewRouter` (`Server/api/router.go:106`) — not in `main.go`. The seven +post-construction setters and where they are called: + +| Setter | Declared | Called from | Required before `Run`? | +| ------------------------- | ---------------------------- | --------------------------------- | ------------------------------------------------------------------------------------------ | +| `SetPluginRegistry` | `ws/hub_events.go:60` | `api/router.go:325` | only when plugins are enabled — optional collaborator | +| `SetPluginEventSink` | `ws/hub_events.go:70` | `api/router.go:328` | same | +| `SetLiveKit` | `ws/hub_livekit.go:10` | `api/router.go:342` | **yes** for voice: every voice join needs the token signer; nil means voice silently fails | +| `SetLiveKitProcess` | `ws/hub_livekit.go:48` | `api/router.go:360` | only when the supervised LiveKit process is configured | +| `SetEventPersister` | `ws/hub_events.go:40` | `main.go:453` | **yes** when persistence is on: events emitted before it is set are not persisted | +| `SetEventStore` | `ws/hub_events.go:48` | `main.go:454` | **yes** for replay: resume without a store answers a full `ready` | +| `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. + +### Locks + +Five locks on `Hub`, all `syncutil` (so the `-tags deadlock` pass sees them): + +| Lock | Declared | Guards (from the field comment) | +| ------------- | --------------- | ----------------------------------------------------------------------- | +| `mu` | `ws/hub.go:25` | the client registry and subscriptions | +| `seqMu` | `ws/hub.go:58` | seq assignment + replay insertion + delivery order, serialised together | +| `settingsMu` | `ws/hub.go:122` | the cached server settings | +| `keyHolderMu` | `ws/hub.go:130` | the voice E2EE key-holder map | +| `presenceMu` | `ws/hub.go:135` | presence coalescing state | + +**The lock order is not written down anywhere** — not in `Server/CLAUDE.md`, +`docs/architecture/websocket.md`, or `hub.go`. It is proven only by the +`-tags deadlock -count=10 ./ws/` pass. B3-5 records the order in `hub.go`'s +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 order, then the `defer` stack that undoes it (LIFO — the last started +is the first stopped): + +| # | Start (`main.go`) | Stop (`defer`, in registration order — runs in reverse) | +| --- | -------------------------------------------------------- | ---------------------------------------------------------- | +| 1 | background context | `bgCancel()` `:118` | +| 2 | `runOpenDatabase` → `db.OpenWithMaxReaders` `:314-322` | `database.Close()` `:148` | +| 3 | `runInitDatabase` → `db.Migrate` `:333-347` | — | +| 4 | `runInitTelemetry` `:369` | `telemetryStop()` `:156` | +| 5 | `runInitPlugins` `:392` | `runClosePlugins` `:161` | +| 6 | `api.NewRouter` → **hub built here** `:164` | `routerCleanup()` `:165`, then `hub.GracefulStop()` `:172` | +| 7 | `runStartEventPersistence` `:435` (sets persister/store) | `runStopEventPersistence` `:176` | +| 8 | `runStartAuditWriter` `:488` | `runStopAuditWriter` `:186` | +| 9 | `runStartACME` `:505` | via `runShutdownServers` `:667` | +| 10 | `runStartMaintenance` `:527` | `maintenanceStop()` `:207` | +| 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 +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. + +## Auth slice — before-state dependency graph + +The three files B3-2 moves, and what they depend on at `ad4defc2`. The +after-state table is appended by B3-2. + +| File | Imports (module-internal) | `db` symbols used | +| ---------------------- | ---------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| `api/auth_handler.go` | `auth`, `db`, `permissions`, `service` | types `DB`, `Session`, `User`; funcs `WriteAudit`, `IsUniqueConstraintError`; sentinels `ErrLastAdmin`, `ErrNotFound`; methods `CreateSession`, `CreateUserWithInvite`, `DeleteAccount`, `DeleteSession`, `GetSetting`, `GetUserByID`, `GetUserByUsername`, `UpdateUserCustomStatus` | +| `api/totp_handler.go` | `auth`, `db` | types `DB`, `Session`, `User`; func `WriteAudit`; methods `DeleteOtherSessions`, `GetUserByID`, `UpdateUserTOTPSecret` | +| `auth/*.go` (10 files) | `db` (types only, in `helpers.go`, `resolve.go`), `config`, `syncutil` | types `User`, `Session`, `APIToken`, `Role` — no method calls; `auth` is a leaf that computes and does not persist | + +Eleven distinct `*db.DB` methods across the two handlers. That is the upper +bound of the interface `api/auth_deps.go` declares in B3-2; the after-state +row must show the handlers importing neither `db` nor `service` directly. + +## Client baselines are not here + +The supplement's Phase 1 item 5 (client native-import, Rust-command, +import-cycle, timer/listener, bundle, coverage and mutation baselines) is B7's +entry work, recorded there. Nothing under `Client/` was measured for this +document. diff --git a/docs/architecture/server.md b/docs/architecture/server.md index 0db0028d..e501a12b 100644 --- a/docs/architecture/server.md +++ b/docs/architecture/server.md @@ -9,6 +9,11 @@ LiveKit for voice, Wazero for plugins (build-tag gated), optional OpenTelemetry ## D2 — Package map +Which of these packages may import `db`, and what every file above the domain +layer does with it, is inventoried in +[server-boundaries.md](server-boundaries.md) (B3-0) and enforced by the +`db-import-boundary` rule in `Server/invariants/`. + ```mermaid flowchart TB subgraph entry ["Process entry"] diff --git a/docs/plans/README.md b/docs/plans/README.md index 8d7844c5..6050bc2d 100644 --- a/docs/plans/README.md +++ b/docs/plans/README.md @@ -10,19 +10,20 @@ 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. | -| [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) in review 2026-08-29. | +| [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 3b731b5b..900bb171 100644 --- a/docs/plans/b3-server-architecture-guardrails-2026-08-29.md +++ b/docs/plans/b3-server-architecture-guardrails-2026-08-29.md @@ -4,8 +4,9 @@ **Base commit:** `bf7b886d` (`dev`, post-PR #1445); HP-2 accepted 2026-08-29 ([hp-2-scorecard-2026-08-29.md](hp-2-scorecard-2026-08-29.md)) — claims verified at `bf7b886d` -**Status:** drafted — entry gate 2 of 3 met at draft time (see below); no step -started. Update this line, not only the step table, when a step lands. +**Status:** in progress — plan merged 2026-08-29 (PR #1447 = `ad4defc2`); +B3-0 in review 2026-08-29 (evidence in its section; closes entry-gate item 3). +Update this line, not only the step table, when a step lands. Primary inputs: @@ -36,14 +37,14 @@ surface to it. | **B3-2** | The auth vertical slice (S-10): route → `service.AuthService` → `db`, behaviour-neutral | 2–3 days | B3-6, B3-7 | | **HP-3** | First vertical-slice review — scorecard | — | — | | **B3-3** | Lifecycle extraction: `main.go` → `internal/app/` with one composite close contract | 1–2 days | B3-4 | -| **B3-4** | Hub constructor options (S-11): required collaborators validated at construction | 1 day | B3-3 | +| **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 | | **B3-7** | Alpha-shaped test dataset: seed profile + anonymised `v1.2.0-alpha.4` snapshot | 1–2 days | B3-0..B3-2 | | **B3-8** | Remaining domain families behind services (S-09), one PR each; S-03/S-04 fold into the channel family | spread | after HP-3, per-family | | **B3-9** | The B3-tagged findings: OC-0323, OC-0345, OC-0346 (test-first, `bughunt-fix` shape) | 1 day | any | -Order: B3-0 → B3-1 → B3-2 → **HP-3** → B3-3 + B3-4 → B3-5 → B3-8. B3-6, B3-7 +Order: B3-0 → B3-1 → B3-2 → **HP-3** → B3-3 → B3-4 → B3-5 → B3-8. B3-6, B3-7 and B3-9 run beside the slice (roadmap "Safe parallelism": guardrail tooling and baseline measurement may run while the first vertical slice is prepared) provided they do not touch `Server/api/auth_handler.go`, `Server/auth/` or @@ -64,21 +65,21 @@ migration, predicate or hub lifecycle ownership. Every claim the roadmap's B3 section and the supplement rest on, re-tested against `bf7b886d`. Commands are the ones B3-0 automates. -| Claim | Verdict | What it means for the work | -| ---------------------------------------------------------------------------- | ------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| 44 production files in `ws`/`admin`/`api` import `db` (17/16/11) | **Confirmed, 45** | `ws` 17, `admin` 16, `api` **12** (`grep -l '"github.com/J3vb/OwnCord/Server/db"'` over non-test files). `service` imports it from 16 of 18 files — expected, it is the layer that should. `auth` from 2 of 10. B3-0 lists every file, not the count. | -| `main.go` exceeds 1,000 lines and owns twelve responsibilities | **Confirmed** | 1,019 lines. B3-3 moves the wiring into `internal/app/`; `main.go` stays the `go build .` entry. | -| `hub_broadcast.go`, `serve.go`, `hub.go` are the coordination hotspots | **Confirmed** | 1,032 / 990 / 819 lines; `router.go` 721, `voice_join.go` 680. `ws` is 45 production files, 12,738 lines. B3-5 splits inside the package — the supplement's rule ("keep `ws` one package while its lock invariants need shared private state") stands. | -| Hub wiring uses post-construction setters (S-11) | **Confirmed, 7** | `SetEventPersister`, `SetEventStore`, `SetPluginRegistry`, `SetPluginEventSink` (`hub_events.go`), `SetLiveKit`, `SetLiveKitProcess` (`hub_livekit.go`), `SetPendingVoiceModFlags` (`voice_moderation.go:599`). B3-4 decides for each: required (constructor option, validated) or genuinely replaceable (setter stays). | -| Auth routes consume raw database ownership (S-10) | **Confirmed** | `api/auth_handler.go` (786 lines) takes `*db.DB` in `MountAuthRoutes`, `handleRegister`, `handleLogin`, `loginAuthenticate`, `handleLogout`, `handleDeleteAccount`, `issueSession`, `isRequire2FAEnabled`, `isRegistrationOpen`, `getBooleanSetting`; 26 `db.` references. `totp_handler.go` 475 lines, same shape. No `service/auth.go` exists. | -| A useful service seam exists but does not own all use cases | **Confirmed** | `Server/service/` has 18 production files (block, channel, dm, emoji, invite, mentions, message\*, moderation, permission, role, user). Nothing for auth, sessions, TOTP, uploads, settings, audit or plugins. | -| Coverage is 74.6% with no floor (S-06) | **Confirmed** | `ci.yml:73` runs `go test -race -coverprofile=coverage.out -cover` and uploads the profile (`:91-96`); nothing reads it. B0 measured 74.6% (`b0-baseline-2026-08-25.md:46`). B3-6 adds the floor at exactly that number. | -| Tier 3 (hub simulation, fault transport, model tests) is designed, not built | **Confirmed** | `bug-detection-improvements.md` §Tier 3; `make fuzz` (Tier 1a) exists (`Server/Makefile:5`). No `ws` simulation test, no fault-injecting transport, no `fc.commands` model test in `Client/tests/unit/*.property.test.ts`. B3-6 builds 3b and 3c; 3a is a client test file and is included (it touches no client structure — B7's rule is about `src/`). | -| `Server/invariants` has rules to extend | **Confirmed, one rule** | `Rules = []Rule{syncutilLocks}` (`invariants.go:64`). The `seq-enqueue-paired` rule the 2026-08-18 measurement recorded as "adopted narrowed" was never merged under that name — `git log -S seq-enqueue-paired` is empty at `bf7b886d` — so B3-6 item 7 adds `authz-chokepoint` as the registry's second rule and does not build on a sibling that does not exist. `authz-chokepoint` gets HP-2 question 5's residue as its allowlist. | -| The seed tool has no alpha-shaped profile (workstream 12) | **Confirmed** | `Server/cmd/seed/main.go` (372 lines) takes `-db` and `-confirm-dev` only. No snapshot exists anywhere in the tree. B3-7 builds both. | -| Docker smoke never runs on `dev` (workstream 16) | **Confirmed** | `Server Docker Build (verify)` skips on every `dev` PR (HP-2 gate run; #1444, #1445 both `skipping`). B3-6 adds a `schedule:` trigger to `ci.yml` scoped to that job. | -| Permission rules are mirrored (workstream 7) | **Refuted, done in B2-5** | Six predicates in `Server/permissions/predicates.go`, parity tables in `service/` and `ws/`; residue classified in HP-2 question 5. Workstream 7 reduces to the `authz-chokepoint` rule (workstream 15) and to keeping the residue table current when B3-8 moves families. | -| Register rows OC-0323, OC-0345, OC-0346 are open | **Confirmed** | All three `open`, low, in `.superpowers/findings-ledger.json`; B3/B5, B3/B4, B3/B6 tags. Roadmap rule 2: B3 cannot exit with any of them open unless re-tagged with a written reason in HP-3. | +| Claim | Verdict | What it means for the work | +| ---------------------------------------------------------------------------- | ------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| 44 production files in `ws`/`admin`/`api` import `db` (17/16/11) | **Confirmed, 45** | `ws` 17, `admin` 16, `api` **12** (`grep -l '"github.com/J3vb/OwnCord/Server/db"'` over non-test files). `service` imports it from 16 of 18 files — expected, it is the layer that should. `auth` from 2 of 10. B3-0 lists every file, not the count. | +| `main.go` exceeds 1,000 lines and owns twelve responsibilities | **Confirmed** | 1,019 lines. B3-3 moves the wiring into `internal/app/`; `main.go` stays the `go build .` entry. | +| `hub_broadcast.go`, `serve.go`, `hub.go` are the coordination hotspots | **Confirmed** | 1,032 / 990 / 819 lines; `router.go` 721, `voice_join.go` 680. `ws` is 45 production files, 12,738 lines. B3-5 splits inside the package — the supplement's rule ("keep `ws` one package while its lock invariants need shared private state") stands. | +| Hub wiring uses post-construction setters (S-11) | **Confirmed, 7** | `SetEventPersister`, `SetEventStore`, `SetPluginRegistry`, `SetPluginEventSink` (`hub_events.go`), `SetLiveKit`, `SetLiveKitProcess` (`hub_livekit.go`), `SetPendingVoiceModFlags` (`voice_moderation.go:599`). Called from two places: `api/router.go` (`NewHub` at `:106`, plugin and LiveKit setters `:325-360`) and `main.go:453-454` (persister, event store) — so construction has two owners today, which is why B3-4 follows B3-3. B3-4 decides for each: required (constructor option, validated) or genuinely replaceable (setter stays). | +| Auth routes consume raw database ownership (S-10) | **Confirmed** | `api/auth_handler.go` (786 lines) takes `*db.DB` in `MountAuthRoutes`, `handleRegister`, `handleLogin`, `loginAuthenticate`, `handleLogout`, `handleDeleteAccount`, `issueSession`, `isRequire2FAEnabled`, `isRegistrationOpen`, `getBooleanSetting`; 26 `db.` references. `totp_handler.go` 475 lines, same shape. No `service/auth.go` exists. | +| A useful service seam exists but does not own all use cases | **Confirmed** | `Server/service/` has 18 production files (block, channel, dm, emoji, invite, mentions, message\*, moderation, permission, role, user). Nothing for auth, sessions, TOTP, uploads, settings, audit or plugins. | +| Coverage is 74.6% with no floor (S-06) | **Confirmed** | `ci.yml:73` runs `go test -race -coverprofile=coverage.out -cover` and uploads the profile (`:91-96`); nothing reads it. B0 measured 74.6% (`b0-baseline-2026-08-25.md:46`). B3-6 adds the floor at exactly that number. | +| Tier 3 (hub simulation, fault transport, model tests) is designed, not built | **Confirmed** | `bug-detection-improvements.md` §Tier 3; `make fuzz` (Tier 1a) exists (`Server/Makefile:5`). No `ws` simulation test, no fault-injecting transport, no `fc.commands` model test in `Client/tests/unit/*.property.test.ts`. B3-6 builds 3b and 3c; 3a is a client test file and is included (it touches no client structure — B7's rule is about `src/`). | +| `Server/invariants` has rules to extend | **Confirmed, one rule** | `Rules = []Rule{syncutilLocks}` (`invariants.go:64`). The `seq-enqueue-paired` rule the 2026-08-18 measurement recorded as "adopted narrowed" was never merged under that name — `git log -S seq-enqueue-paired` is empty at `bf7b886d` — so B3-6 item 7 adds `authz-chokepoint` as the registry's second rule and does not build on a sibling that does not exist. `authz-chokepoint` gets HP-2 question 5's residue as its allowlist. | +| The seed tool has no alpha-shaped profile (workstream 12) | **Confirmed** | `Server/cmd/seed/main.go` (372 lines) takes `-db` and `-confirm-dev` only. No snapshot exists anywhere in the tree. B3-7 builds both. | +| Docker smoke never runs on `dev` (workstream 16) | **Confirmed** | `Server Docker Build (verify)` skips on every `dev` PR (HP-2 gate run; #1444, #1445 both `skipping`). B3-6 adds a `schedule:` trigger to `ci.yml` scoped to that job. | +| Permission rules are mirrored (workstream 7) | **Refuted, done in B2-5** | Six predicates in `Server/permissions/predicates.go`, parity tables in `service/` and `ws/`; residue classified in HP-2 question 5. Workstream 7 reduces to the `authz-chokepoint` rule (workstream 15) and to keeping the residue table current when B3-8 moves families. | +| Register rows OC-0323, OC-0345, OC-0346 are open | **Confirmed** | All three `open`, low, in `.superpowers/findings-ledger.json`; B3/B5, B3/B4, B3/B6 tags. Roadmap rule 2: B3 cannot exit with any of them open unless re-tagged with a written reason in HP-3. | Net effect: workstream 7 is already done; the inventory (entry-gate item 3) is the first real work; the auth slice is exactly the size S-10 says; `api` @@ -136,6 +137,63 @@ Exit: every `db` importer has a disposition; the rule is green on HEAD and red on a synthetic violation; the graph table exists for the three auth files. One PR. +**Evidence, 2026-08-29** — branch `feat/b3-0-boundary-inventory` from `dev` +`ad4defc2`; PR to `dev` recorded below. Closes entry-gate item 3. + +- **Inventory:** `docs/architecture/server-boundaries.md`, linked from + `docs/architecture/server.md` §D2 and `docs/architecture/README.md`. The + table is generated by `Server/cmd/dbinventory` (syntactic, `go/ast`; no + `x/tools` dependency added) and the rows live as `DBImportAllow` in + `Server/invariants/db_import_boundary.go`, so the document and the gate + cannot drift: the tool prints each row's disposition from the map and exits + 1 on an unlisted importer or a stale row. Measured: **51 files** import + `db` outside `db/` and `service/` (ws 17, admin 16, api 12, auth 2, root 2, + cmd/seed 1, plugin 1) — the supplement's 44 counted three packages, and + `api` is 12, not 11. **14 are type-only** (they use `db.User`-style shapes + and never persist) — a distinction the `git grep` count could not make. + Dispositions: **move 28** (auth 9, channel 6, settings-ops 4, voice 3, + upload 2, user 2, role 1, connection 1), **adapter 17**, **boundary 6**, + remove 0. The regex the plan sketched over-counted: `db.` in comments + matched, and `*db.DB` method calls (`database.GetUserByID`) do not contain + `db.` at all — the AST walk resolves `*db.DB` receivers (params, fields, + `db.Open*` assignments) instead. +- **Rule:** `db-import-boundary`, the registry's second rule. Unit RED: + `TestDBImportBoundary` "unlisted api file importing db is flagged" and the + aliased-import case. Real-tree RED, B2-7 style — a probe `api/zz_probe.go` + importing `db`: + + ``` + --- FAIL: TestServerInvariants (0.04s) + invariants_test.go:20: api/zz_probe.go:3: [db-import-boundary] imports Server/db above the domain layer without an inventory row; route the call through a service (see docs/architecture/server-boundaries.md), or add a DBImportAllow entry with a disposition and reason + ``` + + Probe deleted (`git status` clean), `go test ./invariants/` green. + `TestDBImportAllowIsLive` is the other direction: every row must name a + file that exists and still imports `db`, carry a reason, and pair `move` + with a family — so the list can only shrink honestly. + +- **Hub lifecycle:** seven setters with declaration and call site — `NewHub` + is called in `api/router.go:106`, four setters from the router and two + from `main.go:453-454`; four are required before `Run` (LiveKit signer, + persister, event store — and `SetLiveKit` fails voice silently when + absent), three are genuinely optional. Five `syncutil` locks listed with + what each guards; **the lock order is not written down anywhere** — only + the `-tags deadlock` pass proves it — so B3-5 records it in `hub.go` before + its first move. The `run()` start order and its twelve-entry defer stack + are tabulated with the three ordering facts B3-3's composite close must + keep (audit writer after `database.Close`, persistence before it, + `GracefulStop` on early return). +- **Auth before-graph:** `api/auth_handler.go` imports `auth`, `db`, + `permissions`, `service` and calls 8 distinct `*db.DB` methods; + `totp_handler.go` imports `auth`, `db` and calls 3; `auth/` itself is a + leaf (types only). Eleven methods is the upper bound of B3-2's interface. +- Phase 1 item 5 (client baselines) recorded as B7 entry work, not done here. +- Gates before commit: `check:docs`, `check:hygiene`; from `Server/` the four + build-tag variants, `go vet`, `go test -race ./...`, `go test -tags deadlock +./ws/`, `golangci-lint run` (the tool's first version tripped `cyclop` at + 30 and the deprecated `parser.ParseDir` — split into helpers, `os.ReadDir` + - `ParseFile`). + ## B3-1 — Auth characterization tests Layout-refactor Phase 2 item 1 and the supplement's PR strategy step 1 @@ -234,8 +292,11 @@ Roadmap workstream 8; supplement Phase 3 item 1. After HP-3. 1. `Server/internal/app/` owns: config and data-directory preparation, database open/migrate, telemetry, plugin registry, event persistence, - audit writer, maintenance workers, HTTP server construction, health, - replay seeding, and **one composite close** — `App.Close(ctx)` that stops + audit writer, maintenance workers, **hub construction** (moved out of + `api.NewRouter`, `router.go:106`, together with the plugin and LiveKit + setters at `:325-360`; `NewRouter` gains a `*ws.Hub` parameter and stops + returning one), HTTP server construction, health, replay seeding, and + **one composite close** — `App.Close(ctx)` that stops in the reverse of start order and reports the first error without skipping later closes. `main.go` becomes `cfg := …; app, err := app.New(cfg); err = app.Run(ctx)`. @@ -258,9 +319,14 @@ Exit: `main.go` under 150 lines; the failure-injection test green under ## B3-4 — Hub constructor options (S-11) -Roadmap workstream 6; supplement Phase 3 item 2. Parallel with B3-3 (touches -`ws/hub*.go` and the one construction site, not `main.go`'s other blocks — -coordinate the construction-site line). +Roadmap workstream 6; supplement Phase 3 item 2. **After B3-3, not +parallel with it:** at `bf7b886d` the production `ws.NewHub` call is inside +`api.NewRouter` (`Server/api/router.go:106`), which also wires +`SetPluginRegistry`/`SetPluginEventSink` (`:325-328`) and +`SetLiveKit`/`SetLiveKitProcess` (`:342-360`), while `main.go:453-454` sets +the event persister and store after the router returns. Two construction +boundaries means two owners; B3-3 collapses them first (below), then B3-4 +has one call site to change. 1. From B3-0's setter table: each of the seven becomes either a field of `HubOptions` validated in `NewHub` (required → construction fails without @@ -268,8 +334,10 @@ coordinate the construction-site line). is replaceable at runtime (`SetPendingVoiceModFlags` is the likely survivor; `SetLiveKitProcess` depends on whether the supervised process can restart). -2. `NewHub` returns `(*Hub, error)`; the single call site (`main.go`, or - `internal/app/` once B3-3 lands) passes everything it used to set. Tests +2. `NewHub` returns `(*Hub, error)`; the single call site — `internal/app/` + after B3-3, which passes the built `*ws.Hub` into `api.NewRouter` instead + of having the router construct it — passes everything the seven setters + used to set. Tests that build a `Hub` use a `testHubOptions()` helper so 170+ test files do not each grow a struct literal. 3. RED first: a test that constructs a `Hub` without a required collaborator @@ -355,10 +423,23 @@ each item is its own PR so none blocks another. Nothing here edits `permissions.HasPerm` call in `api/`. B3-8 shrinks the allowlist as families move. 8. **Docker smoke nightly on `dev` (workstream 16).** `ci.yml` gains - `schedule: [{cron: "0 3 * * *"}]` and the `Server Docker Build (verify)` - job's condition becomes `github.ref == 'refs/heads/main' || -github.event_name == 'schedule'`; `concurrency` and `timeout-minutes` - already present (B1-7's guard check enforces both). + `schedule: [{cron: "0 3 * * *"}]`. The `Server Docker Build (verify)` + job's condition (`ci.yml:520`, today + `github.ref_name == 'main' || github.base_ref == 'main'`) **keeps both + existing terms** — a PR to `main` runs on the synthetic + `refs/pull//merge` ref and is matched by `base_ref`, not `ref_name` — + and adds `|| github.event_name == 'schedule'`. A scheduled run executes + the workflow file from the default branch (`main`) and the job's + `actions/checkout` has no `ref`, so it would smoke `main`; the checkout + step gains `ref: ${{ github.event_name == 'schedule' && 'dev' || '' }}` + (empty = the event's own ref, unchanged for pushes and PRs). The nightly + therefore builds `dev`'s `Server/` from a workflow definition taken from + `main` — acceptable while the job's steps are identical on both branches, + and the reason the job is not moved to its own workflow file. + `concurrency` and `timeout-minutes` are already present (B1-7's guard + check enforces both). Proof before enabling the schedule: a + `workflow_dispatch` run with the same `ref` expression and a + `git rev-parse HEAD` step showing `dev`'s SHA. 9. **Machine-readable contract drift (workstream 10).** `check:server` already diffs the two generators; this adds `docs/api.md` route-table generation from the mounted router (the absence test's walker, printed as @@ -374,11 +455,20 @@ baseline) in this section's evidence block. Roadmap workstream 12. Beside the slice. -1. `Server/cmd/seed -profile alpha` — deterministic (fixed seed, fixed clock): - member count, channel count, message volume, attachment count, role and - override distribution, DM share and voice-session history matching the - documented alpha shape (numbers from the load-baseline workflow's - parameters, `load-baseline.yml`, so the two agree). `-confirm-dev` stays +1. `Server/cmd/seed -profile alpha` — deterministic (fixed seed, fixed + clock). `load-baseline.yml` defines only `users` (default 100) and one + channel, so it cannot be the source of the shape; the profile is defined + here and lives as constants in `Server/cmd/seed/profile_alpha.go`, and + `load-baseline.yml`'s `users` default is the one value the two share: + **100 users** (4 roles: owner, admin, moderator, member — 1/2/5/92), **12 + channels** (10 text, 2 voice; 3 with role overrides, 2 with user + overrides, 1 archived), **20,000 messages** over 30 simulated days with a + diurnal curve, **300 attachments** (image/audio/video/other 60/10/10/20 %, + sizes 10 KB–5 MB), **15 % of messages in DMs** across 40 DM pairs, + **200 voice sessions** (1–45 min, 2–6 participants), **500 reactions**, + **30 invites** (10 revoked), **1 plugin row** (disabled). A number that + B3-7 finds unrepresentative is changed in `profile_alpha.go` with the + reason in its evidence block — never silently. `-confirm-dev` stays mandatory. 2. One anonymised `v1.2.0-alpha.4` snapshot at `Server/testdata/snapshots/v1.2.0-alpha.4.sqlite` (Git LFS if over 5 MB; the path documented in `docs/deployment.md` diff --git a/docs/plans/repo-health-roadmap-2026-08-23.md b/docs/plans/repo-health-roadmap-2026-08-23.md index 37145866..dc95741d 100644 --- a/docs/plans/repo-health-roadmap-2026-08-23.md +++ b/docs/plans/repo-health-roadmap-2026-08-23.md @@ -1249,7 +1249,10 @@ in this order: (PR #1443 = `88c7a824`) and B2-9 — **done 2026-08-29**; 6. HP-2 — **accepted 2026-08-29** ([hp-2-scorecard-2026-08-29.md](hp-2-scorecard-2026-08-29.md)); B2 is - closed and B3 opens as described below. + closed; +7. B3 — **started 2026-08-29**, execution plan + [b3-server-architecture-guardrails-2026-08-29.md](b3-server-architecture-guardrails-2026-08-29.md); + B3-0 (boundary inventory) is the first step, then the auth slice to HP-3. Do not begin B3 domain extraction, client platform extraction, or browser work before HP-2 closes. When it does, B3 opens with the "First actionable slice"