diff --git a/Server/invariants/db_import_boundary.go b/Server/invariants/db_import_boundary.go index f40f3175..7e8a7972 100644 --- a/Server/invariants/db_import_boundary.go +++ b/Server/invariants/db_import_boundary.go @@ -92,7 +92,8 @@ var DBImportAllow = map[string]DBImportEntry{ "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", "member and presence broadcast payloads read the user and role they announce"}, + "ws/hub_broadcast.go": {"move", "channel", "member broadcast payloads read the user and role they announce"}, + "ws/hub_presence.go": {"adapter", "", "presence coalescer; pure BroadcastStatus helper and the MemberSummary shape"}, "ws/hub_visibility.go": {"move", "channel", "visibility and audience resolution reads channels, overrides, participants, users"}, "ws/hub_sweep.go": {"move", "voice", "stale-voice sweep reads and leaves"}, "ws/messages.go": {"adapter", "", "wire types + pure status helpers"}, diff --git a/Server/ws/hub.go b/Server/ws/hub.go index cfe02c87..90f3726b 100644 --- a/Server/ws/hub.go +++ b/Server/ws/hub.go @@ -566,19 +566,6 @@ func (h *Hub) EventPersisterStats() (persisted, dropped, flushes, errs uint64, o return persisted, dropped, flushes, errs, true } -// VoiceSessionCount returns the number of clients currently in a voice channel. -func (h *Hub) VoiceSessionCount() int { - h.mu.RLock() - defer h.mu.RUnlock() - count := 0 - for _, c := range h.clients { - if c.getVoiceChID() != 0 { - count++ - } - } - return count -} - // topicRateLimitPerSecond is the default maximum messages per second for any // single channel topic. Prevents a busy channel from saturating the broadcast // loop and starving other channels. diff --git a/Server/ws/hub_broadcast.go b/Server/ws/hub_broadcast.go index 922db2f5..9d60170d 100644 --- a/Server/ws/hub_broadcast.go +++ b/Server/ws/hub_broadcast.go @@ -79,69 +79,6 @@ func (h *Hub) BroadcastToAllExcept(excludeUserID int64, msg []byte) { } } -// broadcastVoiceEvent enqueues a voice_state / voice_leave message for the -// connected clients whose current role may READ channelID. -// -// These events used to go out via BroadcastToAll, which handed every -// authenticated client the membership and camera/mute state of voice channels -// that channel_overrides hides from their role — while the equivalent read path -// (buildReady) deliberately filters voice states to readable channels. Tagging -// the event with its real channel id also makes reconnect replay filter it, -// where a channelID of 0 was replayed unconditionally. -// -// The audience is resolved here, on the caller's goroutine, so the hub's -// dispatch loop never blocks on permission lookups. -func (h *Hub) broadcastVoiceEvent(ctx context.Context, channelID int64, msg []byte) { - // A room's own participants must always receive its voice_state / - // voice_leave: voice membership is gated on CONNECT_VOICE alone, so the - // READ filter can exclude a live participant — whose client then keeps a - // stale E2EE key holder, stalling rotation and locking new joiners out - // until e2ee_timeout. Union the READ audience with the room's current - // participants; what outsiders may observe is unchanged. - audience := h.channelReadAudience(ctx, channelID) - seen := make(map[int64]struct{}, len(audience)) - for _, uid := range audience { - seen[uid] = struct{}{} - } - h.mu.RLock() - for uid, c := range h.clients { - if _, ok := seen[uid]; !ok && c.getVoiceChID() == channelID { - audience = append(audience, uid) - } - } - h.mu.RUnlock() - h.broadcastChannelScopedTo(channelID, msg, audience, "voice event") -} - -// broadcastVoiceEventWithLeaver is broadcastVoiceEvent extended to guarantee -// leaverID is in the audience even though the caller has already cleared -// their client-side voice state — which means broadcastVoiceEvent's own -// still-in-the-room participant union can no longer see them. Every path -// that tears down a voice participant whose client state is cleared before -// the voice_leave goes out needs this: voice membership is gated on -// CONNECT_VOICE alone, so a leaver without READ_MESSAGES on the channel -// would otherwise never learn the server already ended their call. Mirrors -// CleanupVoiceForChannel's per-batch leaver union, for the single-leaver case. -func (h *Hub) broadcastVoiceEventWithLeaver(ctx context.Context, channelID int64, msg []byte, leaverID int64) { - audience := h.channelReadAudience(ctx, channelID) - seen := make(map[int64]struct{}, len(audience)+1) - for _, uid := range audience { - seen[uid] = struct{}{} - } - h.mu.RLock() - for uid, c := range h.clients { - if _, ok := seen[uid]; !ok && c.getVoiceChID() == channelID { - seen[uid] = struct{}{} - audience = append(audience, uid) - } - } - h.mu.RUnlock() - if _, ok := seen[leaverID]; !ok { - audience = append(audience, leaverID) - } - h.broadcastChannelScopedTo(channelID, msg, audience, "voice event") -} - // broadcastChannelScoped enqueues msg for exactly the connected clients whose // current role may READ channelID, tagged with that channel id so reconnect // replay filters it too (EventsSinceFiltered replays a channelID of 0 @@ -303,151 +240,6 @@ func (h *Hub) BroadcastUserUpdate(u UserUpdate) { h.BroadcastToAll(buildUserUpdate(u)) } -// presenceCoalesceWindow is how long QueuePresence buffers connect/disconnect -// presence before flushing. Long enough to collapse a socket flap -// (disconnect+reconnect through a proxy blip) into one frame, short enough -// that a genuine arrival still looks immediate to humans. -const presenceCoalesceWindow = 300 * time.Millisecond - -// pendingPresence is the coalescer's latest-wins entry for one user. -type pendingPresence struct { - status string - customStatus *string -} - -// QueuePresence coalesces connect/disconnect presence broadcasts: the latest -// state per user is buffered for presenceCoalesceWindow and then flushed via -// BroadcastPresence. Each un-coalesced presence change is a sequenced global -// broadcast — an O(connected clients) fan-out under seqMu — so a reconnect -// storm (proxy blip, deploy, network hiccup) used to fire O(users) of them -// from the connect critical path all at once. Latest-wins is exactly -// presence's semantics: a flap inside the window collapses to its final -// state, and the flushed frames are ordinary sequenced presence messages, so -// the wire format and replay behaviour are unchanged. User-chosen status -// changes (presence_update handler) do not pass through here. -func (h *Hub) QueuePresence(userID int64, status string, customStatus *string) { - h.presenceMu.Lock() - if h.presenceQueue == nil { - h.presenceQueue = make(map[int64]pendingPresence) - } - h.presenceQueue[userID] = pendingPresence{status: status, customStatus: customStatus} - armed := h.presenceFlushArmed - h.presenceFlushArmed = true - h.presenceMu.Unlock() - if !armed { - time.AfterFunc(presenceCoalesceWindow, h.flushPresenceQueue) - } -} - -// dropQueuedPresenceAndBroadcast atomically removes any coalesced presence -// still queued for userID and runs broadcast, both under presenceMu. Called -// when a fresher presence for that user is delivered directly (the -// presence_update handler path, via EmitEvents), so the delete and the send -// of the fresher frame can never straddle flushPresenceQueue's own -// snapshot-and-broadcast critical section (OC-0005). -// -// Holding presenceMu across the delete AND the broadcast — rather than just -// the delete — is what actually closes the race: whichever of this call and -// flushPresenceQueue acquires presenceMu second also enqueues its broadcast -// second. -// - If this call goes first, it deletes the entry before flush can ever -// snapshot it, so flush never broadcasts the stale state at all. -// - If flush goes first, this call's delete is a no-op against the -// already-cleared queue, but its broadcast still cannot run until flush's -// own broadcast has already been enqueued — so the fresher frame is -// stamped with the higher seq by deliverBroadcast's single FIFO consumer -// and every client's final view converges on it, not the stale one. -// -// broadcast runs with presenceMu held: every current caller (BroadcastToAll, -// BroadcastToAllExcept) only enqueues onto h.broadcast's non-blocking -// channel send, so this cannot block and introduces no new lock-order edge. -// Both callers sharing that same channel also means the "enqueues second" -// ordering guarantee above translates directly into delivery order: both -// broadcasts are drained by the same single-consumer hub dispatch loop -// (deliverBroadcast), in the order they were enqueued. -func (h *Hub) dropQueuedPresenceAndBroadcast(userID int64, broadcast func()) { - h.presenceMu.Lock() - defer h.presenceMu.Unlock() - delete(h.presenceQueue, userID) - broadcast() -} - -// presenceFlushRaceHook, when non-nil, runs once per flushPresenceQueue call -// immediately after the coalesced queue has been snapshotted and cleared, -// while presenceMu is still held. Test-only (always nil in production): the -// snapshot-to-broadcast window is too narrow to land a real concurrent -// dropQueuedPresenceAndBroadcast reliably, so tests use this hook to -// reproduce that interleaving deterministically. Mirrors the established -// refreshChannelVisibilityRaceHook / voiceJoinPostTokenRaceHook pattern. -// -// It is handed the Hub being flushed, and a test that installs it MUST -// ignore calls for any other Hub. The hook is package-global but flushes are -// per-Hub, and QueuePresence's AfterFunc outlives the test that armed it: -// dropQueuedPresenceAndBroadcast clears the queue without disarming the -// timer, so a sibling test's 300ms flush fires long after that test returned -// — into whatever hook is installed by then. Passing the Hub is what lets the -// installer tell its own flush from that stray one; without it, a hook body -// that is only safe to run once (closing a channel, say) panics. -var presenceFlushRaceHook func(*Hub) - -// flushPresenceQueue drains the coalescer and broadcasts each user's latest -// presence, all under presenceMu (OC-0005). Runs on the AfterFunc timer -// goroutine. -// -// presenceMu is held across the broadcast loop, not just the snapshot: it -// used to be released beforehand, which let a concurrent -// dropQueuedPresenceAndBroadcast (nee dropQueuedPresence) call race in after -// the snapshot had already escaped the lock. The drop was then a guaranteed -// no-op against the live (already-nilled) map, AND nothing constrained -// whether that call's own fresher broadcast landed on h.broadcast before or -// after this loop's stale one — so the stale connect-time presence could win -// the seq race and permanently overwrite a user-chosen status. Holding the -// lock here forces the two critical sections to serialize, which is what -// dropQueuedPresenceAndBroadcast's ordering guarantee depends on. -func (h *Hub) flushPresenceQueue() { - h.presenceMu.Lock() - defer h.presenceMu.Unlock() - queued := h.presenceQueue - h.presenceQueue = nil - h.presenceFlushArmed = false - if presenceFlushRaceHook != nil { - presenceFlushRaceHook(h) - } - for uid, p := range queued { - h.BroadcastPresence(uid, p.status, p.customStatus) - } -} - -// BroadcastPresence fans a presence change out with the invisible mapping -// applied: everyone else sees db.BroadcastStatus(status), the user themselves -// sees the truth. It is the non-handler counterpart of presenceEvents, used by -// the connect and disconnect paths (via the QueuePresence coalescer, which -// delivers through here). -func (h *Hub) BroadcastPresence(userID int64, status string, customStatus *string) { - public := db.BroadcastStatus(status) - if public == status { - h.BroadcastToAll(buildPresenceMsg(userID, status, customStatus)) - return - } - // The public frame's status already collapsed to db.BroadcastStatus, but - // customStatus does not: passing it through verbatim would tell every - // other client an "offline" member's real free-text status, which is a - // tell that they are actually online. Blank it explicitly (not omitted — - // presencePayload.CustomStatus has no omitempty) so the client clears any - // cached text, matching what db.MemberSummary.ForViewer already does for - // the ready payload's member list. - // - // Normal priority, excluding the owner (BroadcastToAllExcept), not - // broadcastExcludeLow: the low-priority queue is unsequenced and dropped - // (not disconnected) on overflow, so it could silently lose this frame - // with no replay recovery, and — since writePump always drains normal - // strictly before low — deliver it out of order against the very - // connect/disconnect presence frames this same coalescer flush also - // produces for other users via BroadcastToAll (OC-0003). - h.BroadcastToAllExcept(userID, buildPresenceMsg(userID, public, nil)) - h.SendToUser(userID, buildPresenceMsg(userID, status, customStatus)) -} - // BroadcastMemberUpdate sends a member_update message to all connected clients // and re-evaluates the reassigned user's live channel subscriptions. func (h *Hub) BroadcastMemberUpdate(userID int64, roleName string) { diff --git a/Server/ws/hub_presence.go b/Server/ws/hub_presence.go new file mode 100644 index 00000000..63525481 --- /dev/null +++ b/Server/ws/hub_presence.go @@ -0,0 +1,153 @@ +package ws + +import ( + "time" + + "github.com/J3vb/OwnCord/Server/db" +) + +// pendingPresence is the coalescer's latest-wins entry for one user. +type pendingPresence struct { + status string + customStatus *string +} + +// QueuePresence coalesces connect/disconnect presence broadcasts: the latest +// state per user is buffered for presenceCoalesceWindow and then flushed via +// BroadcastPresence. Each un-coalesced presence change is a sequenced global +// broadcast — an O(connected clients) fan-out under seqMu — so a reconnect +// storm (proxy blip, deploy, network hiccup) used to fire O(users) of them +// from the connect critical path all at once. Latest-wins is exactly +// presence's semantics: a flap inside the window collapses to its final +// state, and the flushed frames are ordinary sequenced presence messages, so +// the wire format and replay behaviour are unchanged. User-chosen status +// changes (presence_update handler) do not pass through here. +func (h *Hub) QueuePresence(userID int64, status string, customStatus *string) { + h.presenceMu.Lock() + if h.presenceQueue == nil { + h.presenceQueue = make(map[int64]pendingPresence) + } + h.presenceQueue[userID] = pendingPresence{status: status, customStatus: customStatus} + armed := h.presenceFlushArmed + h.presenceFlushArmed = true + h.presenceMu.Unlock() + if !armed { + time.AfterFunc(presenceCoalesceWindow, h.flushPresenceQueue) + } +} + +// dropQueuedPresenceAndBroadcast atomically removes any coalesced presence +// still queued for userID and runs broadcast, both under presenceMu. Called +// when a fresher presence for that user is delivered directly (the +// presence_update handler path, via EmitEvents), so the delete and the send +// of the fresher frame can never straddle flushPresenceQueue's own +// snapshot-and-broadcast critical section (OC-0005). +// +// Holding presenceMu across the delete AND the broadcast — rather than just +// the delete — is what actually closes the race: whichever of this call and + +// presenceCoalesceWindow is how long QueuePresence buffers connect/disconnect +// presence before flushing. Long enough to collapse a socket flap +// (disconnect+reconnect through a proxy blip) into one frame, short enough +// that a genuine arrival still looks immediate to humans. +const presenceCoalesceWindow = 300 * time.Millisecond + +// presenceFlushRaceHook, when non-nil, runs once per flushPresenceQueue call +// immediately after the coalesced queue has been snapshotted and cleared, +// while presenceMu is still held. Test-only (always nil in production): the +// snapshot-to-broadcast window is too narrow to land a real concurrent +// dropQueuedPresenceAndBroadcast reliably, so tests use this hook to +// reproduce that interleaving deterministically. Mirrors the established +// refreshChannelVisibilityRaceHook / voiceJoinPostTokenRaceHook pattern. +// +// It is handed the Hub being flushed, and a test that installs it MUST +// ignore calls for any other Hub. The hook is package-global but flushes are +// per-Hub, and QueuePresence's AfterFunc outlives the test that armed it: +// dropQueuedPresenceAndBroadcast clears the queue without disarming the +// timer, so a sibling test's 300ms flush fires long after that test returned +// — into whatever hook is installed by then. Passing the Hub is what lets the +// installer tell its own flush from that stray one; without it, a hook body +// that is only safe to run once (closing a channel, say) panics. +var presenceFlushRaceHook func(*Hub) + +// flushPresenceQueue acquires presenceMu second also enqueues its broadcast +// second. +// - If this call goes first, it deletes the entry before flush can ever +// snapshot it, so flush never broadcasts the stale state at all. +// - If flush goes first, this call's delete is a no-op against the +// already-cleared queue, but its broadcast still cannot run until flush's +// own broadcast has already been enqueued — so the fresher frame is +// stamped with the higher seq by deliverBroadcast's single FIFO consumer +// and every client's final view converges on it, not the stale one. +// +// broadcast runs with presenceMu held: every current caller (BroadcastToAll, +// BroadcastToAllExcept) only enqueues onto h.broadcast's non-blocking +// channel send, so this cannot block and introduces no new lock-order edge. +// Both callers sharing that same channel also means the "enqueues second" +// ordering guarantee above translates directly into delivery order: both +// broadcasts are drained by the same single-consumer hub dispatch loop +// (deliverBroadcast), in the order they were enqueued. +func (h *Hub) dropQueuedPresenceAndBroadcast(userID int64, broadcast func()) { + h.presenceMu.Lock() + defer h.presenceMu.Unlock() + delete(h.presenceQueue, userID) + broadcast() +} + +// flushPresenceQueue drains the coalescer and broadcasts each user's latest +// presence, all under presenceMu (OC-0005). Runs on the AfterFunc timer +// goroutine. +// +// presenceMu is held across the broadcast loop, not just the snapshot: it +// used to be released beforehand, which let a concurrent +// dropQueuedPresenceAndBroadcast (nee dropQueuedPresence) call race in after +// the snapshot had already escaped the lock. The drop was then a guaranteed +// no-op against the live (already-nilled) map, AND nothing constrained +// whether that call's own fresher broadcast landed on h.broadcast before or +// after this loop's stale one — so the stale connect-time presence could win +// the seq race and permanently overwrite a user-chosen status. Holding the +// lock here forces the two critical sections to serialize, which is what +// dropQueuedPresenceAndBroadcast's ordering guarantee depends on. +func (h *Hub) flushPresenceQueue() { + h.presenceMu.Lock() + defer h.presenceMu.Unlock() + queued := h.presenceQueue + h.presenceQueue = nil + h.presenceFlushArmed = false + if presenceFlushRaceHook != nil { + presenceFlushRaceHook(h) + } + for uid, p := range queued { + h.BroadcastPresence(uid, p.status, p.customStatus) + } +} + +// BroadcastPresence fans a presence change out with the invisible mapping +// applied: everyone else sees db.BroadcastStatus(status), the user themselves +// sees the truth. It is the non-handler counterpart of presenceEvents, used by +// the connect and disconnect paths (via the QueuePresence coalescer, which +// delivers through here). +func (h *Hub) BroadcastPresence(userID int64, status string, customStatus *string) { + public := db.BroadcastStatus(status) + if public == status { + h.BroadcastToAll(buildPresenceMsg(userID, status, customStatus)) + return + } + // The public frame's status already collapsed to db.BroadcastStatus, but + // customStatus does not: passing it through verbatim would tell every + // other client an "offline" member's real free-text status, which is a + // tell that they are actually online. Blank it explicitly (not omitted — + // presencePayload.CustomStatus has no omitempty) so the client clears any + // cached text, matching what db.MemberSummary.ForViewer already does for + // the ready payload's member list. + // + // Normal priority, excluding the owner (BroadcastToAllExcept), not + // broadcastExcludeLow: the low-priority queue is unsequenced and dropped + // (not disconnected) on overflow, so it could silently lose this frame + // with no replay recovery, and — since writePump always drains normal + // strictly before low — deliver it out of order against the very + // connect/disconnect presence frames this same coalescer flush also + // produces for other users via BroadcastToAll (OC-0003). + h.BroadcastToAllExcept(userID, buildPresenceMsg(userID, public, nil)) + h.SendToUser(userID, buildPresenceMsg(userID, status, customStatus)) +} diff --git a/Server/ws/voice_broadcast.go b/Server/ws/voice_broadcast.go index 31088bef..0a6f7aac 100644 --- a/Server/ws/voice_broadcast.go +++ b/Server/ws/voice_broadcast.go @@ -1,6 +1,7 @@ package ws import ( + "context" "time" ) @@ -36,3 +37,79 @@ func qualityBitrate(quality string) int { } return voiceQualities["medium"] } + +// broadcastVoiceEvent enqueues a voice_state / voice_leave message for the +// connected clients whose current role may READ channelID. +// +// These events used to go out via BroadcastToAll, which handed every +// authenticated client the membership and camera/mute state of voice channels +// that channel_overrides hides from their role — while the equivalent read path +// (buildReady) deliberately filters voice states to readable channels. Tagging +// the event with its real channel id also makes reconnect replay filter it, +// where a channelID of 0 was replayed unconditionally. +// +// The audience is resolved here, on the caller's goroutine, so the hub's +// dispatch loop never blocks on permission lookups. +func (h *Hub) broadcastVoiceEvent(ctx context.Context, channelID int64, msg []byte) { + // A room's own participants must always receive its voice_state / + // voice_leave: voice membership is gated on CONNECT_VOICE alone, so the + // READ filter can exclude a live participant — whose client then keeps a + // stale E2EE key holder, stalling rotation and locking new joiners out + // until e2ee_timeout. Union the READ audience with the room's current + // participants; what outsiders may observe is unchanged. + audience := h.channelReadAudience(ctx, channelID) + seen := make(map[int64]struct{}, len(audience)) + for _, uid := range audience { + seen[uid] = struct{}{} + } + h.mu.RLock() + for uid, c := range h.clients { + if _, ok := seen[uid]; !ok && c.getVoiceChID() == channelID { + audience = append(audience, uid) + } + } + h.mu.RUnlock() + h.broadcastChannelScopedTo(channelID, msg, audience, "voice event") +} + +// broadcastVoiceEventWithLeaver is broadcastVoiceEvent extended to guarantee +// leaverID is in the audience even though the caller has already cleared +// their client-side voice state — which means broadcastVoiceEvent's own +// still-in-the-room participant union can no longer see them. Every path +// that tears down a voice participant whose client state is cleared before +// the voice_leave goes out needs this: voice membership is gated on +// CONNECT_VOICE alone, so a leaver without READ_MESSAGES on the channel +// would otherwise never learn the server already ended their call. Mirrors +// CleanupVoiceForChannel's per-batch leaver union, for the single-leaver case. +func (h *Hub) broadcastVoiceEventWithLeaver(ctx context.Context, channelID int64, msg []byte, leaverID int64) { + audience := h.channelReadAudience(ctx, channelID) + seen := make(map[int64]struct{}, len(audience)+1) + for _, uid := range audience { + seen[uid] = struct{}{} + } + h.mu.RLock() + for uid, c := range h.clients { + if _, ok := seen[uid]; !ok && c.getVoiceChID() == channelID { + seen[uid] = struct{}{} + audience = append(audience, uid) + } + } + h.mu.RUnlock() + if _, ok := seen[leaverID]; !ok { + audience = append(audience, leaverID) + } + h.broadcastChannelScopedTo(channelID, msg, audience, "voice event") +} + +// VoiceSessionCount returns the number of clients currently in a voice channel. +func (h *Hub) VoiceSessionCount() int { + h.mu.RLock() + defer h.mu.RUnlock() + count := 0 + for _, c := range h.clients { + if c.getVoiceChID() != 0 { + count++ + } + } + return count +} diff --git a/docs/architecture/server-boundaries.md b/docs/architecture/server-boundaries.md index 4c63186a..2f0b80ed 100644 --- a/docs/architecture/server-boundaries.md +++ b/docs/architecture/server-boundaries.md @@ -15,7 +15,12 @@ row — that family makes no `db` use); 2026-08-31 (B3-5, third split PR) — the first table, on `feat/b3-5-ws-split-3` (the visibility responsibility gathered from `hub_broadcast.go`, `serve.go` and `hub.go` into a new `ws/hub_visibility.go` row; `hub_broadcast.go`'s row now carries only the -member/presence payload reads). +member/presence payload reads); 2026-08-31 (B3-5, fourth split PR) — the +first table, on `feat/b3-5-ws-split-4` (voice broadcast leftovers joined +`voice_broadcast.go`'s code with no row change — that file makes no `db` +use — and the presence coalescer split into a new `ws/hub_presence.go` +adapter row, its only `db` use the pure `BroadcastStatus` helper; +`hub_broadcast.go`'s row is down to the member payload reads). **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 @@ -34,7 +39,7 @@ happens to that use — one of four dispositions from the | 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 | +| `adapter` | a transport adapter that uses `db` types or pure helpers only — response shapes, status helpers — no persistence call | 18 | | `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 | @@ -123,7 +128,8 @@ which is a row worth reading, and none exists today. | `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×2` `Emoji` `Role` | `BroadcastStatus()` | `GetRoleForUser` `GetUserByID` | calls | move | channel | member and presence broadcast payloads read the user and role they announce | +| `ws/hub_broadcast.go` | `Channel×2` `Emoji` `Role` | — | `GetRoleForUser` `GetUserByID` | calls | move | channel | member broadcast payloads read the user and role they announce | +| `ws/hub_presence.go` | — | `BroadcastStatus()` | — | calls | adapter | — | presence coalescer; pure BroadcastStatus helper and the MemberSummary shape | | `ws/hub_sweep.go` | `User` | — | `GetAllVoiceStates` `GetChannel` `GetChannelVoiceStates` `GetSessionsWithBanStatusBatch` `LeaveVoiceChannelIfMatch×2` | calls | move | voice | stale-voice sweep reads and leaves | | `ws/hub_visibility.go` | `Channel×2` `ChannelOverride` `DB` `User` | — | `GetChannel×2` `GetChannelOverridesFor` `GetDMParticipantIDs` `GetRoleByID` `GetUserByID` `GetUserDMChannelIDs` `ListChannels×2` | calls | move | channel | visibility and audience resolution reads channels, overrides, participants, users | | `ws/messages.go` | `Channel×4` `DMChannelInfo×2` `DMUser×2` `Emoji` `Role×3` `User×2` `VoiceState` | `BroadcastStatus()` `StatusForViewer()` | — | calls | adapter | — | wire types + pure status helpers | @@ -135,8 +141,8 @@ which is a row worth reading, and none exists today. | `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 | -58 files import `db` outside `db/` and `service/` (. 1, admin 16, api 10, auth 2, cmd/gendocs 1, cmd/seed 2, internal/app 6, plugin 1, ws 19); 19 are type-only; 0 unlisted. -Dispositions: adapter 17, boundary 13, move 28. Move targets: auth 7, channel 7, connection 2, role 1, settings-ops 4, upload 2, user 2, voice 3. +59 files import `db` outside `db/` and `service/` (. 1, admin 16, api 10, auth 2, cmd/gendocs 1, cmd/seed 2, internal/app 6, plugin 1, ws 20); 19 are type-only; 0 unlisted. +Dispositions: adapter 18, boundary 13, move 28. Move targets: auth 7, channel 7, connection 2, role 1, settings-ops 4, upload 2, user 2, voice 3. diff --git a/docs/plans/README.md b/docs/plans/README.md index 43185d9c..2ef0cff9 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. **Amended 2026-08-31:** BPR-032 reworded to the slim single-current-epoch policy (owner decision; the scope HP-2 accepted). | -| [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 in progress** — the B3 plan row below tracks it. B4–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. **Amended 2026-08-31:** B1 exit reworded to identical-tree integration evidence; B6/B8/B10 `server-info` and epoch-window lines aligned with the B2-2 slim decision; B10 gains the BPR-051 comprehension-read row (owner decisions 2026-08-31). | -| [repo-health-issue-register-2026-08-23](repo-health-issue-register-2026-08-23.md) | 116 planning rows. Public-safe; not a replacement for the ledger. **Amended 2026-08-31:** OC-0349–0375 and OC-0379 enumerated with owner-approved phases; G-03 closure evidence reworded to identical-tree integration evidence; BG-07 re-scoped to BPR-032 as amended. | -| [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/`) merged 2026-08-30 (#1464); B3-4 (hub constructor options) merged 2026-08-31 (#1470); B3-7 (deterministic alpha dataset) merged 2026-08-31 (#1469); B3-5 (`ws` split) in progress — split PR 1 (handshake auth + fresh-connect) merged 2026-08-31 (#1472); split PR 2 (replay family + connection registry) merged 2026-08-31 (#1473); split PR 3 (visibility gather) opened 2026-08-31. | -| [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. **Amended 2026-08-31:** BPR-032 reworded to the slim single-current-epoch policy (owner decision; the scope HP-2 accepted). | +| [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 in progress** — the B3 plan row below tracks it. B4–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. **Amended 2026-08-31:** B1 exit reworded to identical-tree integration evidence; B6/B8/B10 `server-info` and epoch-window lines aligned with the B2-2 slim decision; B10 gains the BPR-051 comprehension-read row (owner decisions 2026-08-31). | +| [repo-health-issue-register-2026-08-23](repo-health-issue-register-2026-08-23.md) | 116 planning rows. Public-safe; not a replacement for the ledger. **Amended 2026-08-31:** OC-0349–0375 and OC-0379 enumerated with owner-approved phases; G-03 closure evidence reworded to identical-tree integration evidence; BG-07 re-scoped to BPR-032 as amended. | +| [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/`) merged 2026-08-30 (#1464); B3-4 (hub constructor options) merged 2026-08-31 (#1470); B3-7 (deterministic alpha dataset) merged 2026-08-31 (#1469); B3-5 (`ws` split) in progress — split PR 1 (handshake auth + fresh-connect) merged 2026-08-31 (#1472); split PR 2 (replay family + connection registry) merged 2026-08-31 (#1473); split PR 3 (visibility gather) merged 2026-08-31 (#1474); split PR 4 (voice leftovers + presence coalescer) opened 2026-08-31. | +| [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 29aa0b43..214e91af 100644 --- a/docs/plans/b3-server-architecture-guardrails-2026-08-29.md +++ b/docs/plans/b3-server-architecture-guardrails-2026-08-29.md @@ -896,6 +896,35 @@ delivery and backpressure — the latter satisfied by what PR decides what else moves), `serve.go` 230 → 183, `hub.go` 616 → 585, `hub_visibility.go` 487. +**Evidence — split PR 4 of the series, 2026-08-31** — branch +`feat/b3-5-ws-split-4` from `dev` `70875a3f` (split PR 3's squash). +Responsibility 7 (voice leftovers) plus the presence coalescer split +that responsibility 6's size target needed: + +- **Move 6** — `broadcastVoiceEvent`, `broadcastVoiceEventWithLeaver` + (from `hub_broadcast.go`) and `VoiceSessionCount` (from `hub.go`) → + the existing `voice_broadcast.go`, beside the voice rate-limit and + quality tables (76 lines out, 77 in). Residue: one `context` import. + Gate: `-tags deadlock -count=10 ./ws/` ok 648.9 s, `-race` ok 228.5 s. +- **Move 7** — the presence coalescer (`pendingPresence`, + `QueuePresence`, `dropQueuedPresenceAndBroadcast`, + `presenceCoalesceWindow`, `presenceFlushRaceHook`, + `flushPresenceQueue`, `BroadcastPresence`) → new `hub_presence.go`, in + source order (145 lines out, 153 in). Residue: scaffolding plus + duplicates of two imports the source keeps (`time`, `db`). Presence is + not one of the plan's seven responsibilities; the cut is what brings + `hub_broadcast.go` to its < 500 exit figure while keeping it pure + delivery and backpressure, and the coalescer is a coherent family of + its own. Gate results in the PR's test plan. +- **Inventory**: `voice_broadcast.go` needs no row (no `db` use); + `hub_presence.go` is an `adapter` row — its only `db` use is the pure + `BroadcastStatus` helper, the doc's own example (17 → 18 `adapter`); + `hub_broadcast.go`'s reason drops the presence half. +- **Sizes**: `hub_broadcast.go` 632 → **424 (< 500 exit target met)**, + `hub.go` 585 → 572 (< 400 still open — the finisher PR moves + construction/options and the replay-limit accessor), `voice_broadcast.go` + 38 → 115, `hub_presence.go` 153. + ## B3-6 — Permanent guardrails Roadmap workstreams 1, 2, 3, 10, 11, 13, 14, 15, 16. Runs beside B3-0..B3-2;