From 1fbedb404ca0ccd16516e547fcab46edb77fc9a5 Mon Sep 17 00:00:00 2001 From: J3vb <192430104+J3vb@users.noreply.github.com> Date: Sat, 18 Jul 2026 12:55:22 +0200 Subject: [PATCH] fix(lint): delete dead ws broadcast cluster; demote phase-header comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit golangci-lint had been failing invisibly behind the earlier CI gate failures. Default-build lint is now clean: - Delete the unused pre-topic-limiter rate-limit constants, the unused bluemonday sanitizer, and the dead broadcast variants superseded by their Low/High counterparts (broadcastExclude, broadcastToDMParticipants(+Exclude), sendSequencedToUsers, PubSub.debugDump). Test references were comments only; updated to name the live variants. - Separate 'Phase X Step Y' file headers from the package clause with a blank line so staticcheck ST1000 no longer reads them as malformed package comments (proper package docs exist in hub.go/manifest.go). - Add .gitattributes normalizing line endings to LF on checkout — the Windows CI runner materialized CRLF, which made every prettier-formatted file fail the format gate. Known remainder (pre-existing, out of P0 scope): golangci-lint with -tags wazero reports 3 gosec + 2 staticcheck and -tags otel 1+1; CI lints the default build. Tracked for the P1 plugin pass. Co-Authored-By: Claude Fable 5 --- .gitattributes | 9 +++++ Server/plugin/host_storage.go | 1 + Server/plugin/host_ui.go | 1 + Server/plugin/loader.go | 1 + Server/plugin/manifest_nottoml.go | 1 + Server/plugin/registry.go | 1 + Server/plugin/sandbox_default.go | 1 + Server/ws/dm_handlers_test.go | 2 +- Server/ws/emit_test.go | 2 +- Server/ws/event_pruner.go | 1 + Server/ws/handlers.go | 67 +++---------------------------- Server/ws/handlers_command.go | 1 + Server/ws/handlers_test.go | 2 +- Server/ws/hub.go | 23 ++--------- Server/ws/pubsub.go | 14 ------- 15 files changed, 28 insertions(+), 99 deletions(-) create mode 100644 .gitattributes diff --git a/.gitattributes b/.gitattributes new file mode 100644 index 00000000..bb26545e --- /dev/null +++ b/.gitattributes @@ -0,0 +1,9 @@ +# Normalize line endings: store LF in the repo and check out LF on every +# platform, so prettier/gofmt see identical bytes locally and in CI. +* text=auto eol=lf + +# Binary assets +*.png binary +*.ico binary +*.wasm binary +*.exe binary diff --git a/Server/plugin/host_storage.go b/Server/plugin/host_storage.go index 812bbed9..ccd15047 100644 --- a/Server/plugin/host_storage.go +++ b/Server/plugin/host_storage.go @@ -3,6 +3,7 @@ // Plugins get a per-plugin namespaced KV store backed by the PluginStore // rows in the events/plugin schema. Capacity caps and value-size caps are // enforced here so a misbehaving plugin can't fill the database. + package plugin import ( diff --git a/Server/plugin/host_ui.go b/Server/plugin/host_ui.go index a4045a0f..453c7b6a 100644 --- a/Server/plugin/host_ui.go +++ b/Server/plugin/host_ui.go @@ -3,6 +3,7 @@ // A plugin that declares the `ui` capability ships HTML/CSS/JS assets and a // list of tabs. The host serves those assets at /api/v1/plugins//ui/... // and the Solid.js client bridge renders each tab inside a sandboxed iframe. + package plugin import ( diff --git a/Server/plugin/loader.go b/Server/plugin/loader.go index 458660e0..848daf5e 100644 --- a/Server/plugin/loader.go +++ b/Server/plugin/loader.go @@ -13,6 +13,7 @@ // // Loader walks the directory, parses every plugin.json, and returns a slice // of foundPlugin records. The Registry then persists each into the store. + package plugin import ( diff --git a/Server/plugin/manifest_nottoml.go b/Server/plugin/manifest_nottoml.go index 9a6938c2..248ad5f6 100644 --- a/Server/plugin/manifest_nottoml.go +++ b/Server/plugin/manifest_nottoml.go @@ -1,6 +1,7 @@ //go:build !wazero // Default build stub — TOML manifest parsing is not compiled in without -tags wazero. + package plugin // tryLoadPluginTOML always reports "not present" in the default build so the diff --git a/Server/plugin/registry.go b/Server/plugin/registry.go index 3ecfafcb..a7ac9583 100644 --- a/Server/plugin/registry.go +++ b/Server/plugin/registry.go @@ -9,6 +9,7 @@ // directory and persists each manifest into the PluginStore so admins can see // what is "installed", but the .wasm files are NOT executed. Calling // Dispatch() in the default build returns ErrRuntimeUnavailable. + package plugin import ( diff --git a/Server/plugin/sandbox_default.go b/Server/plugin/sandbox_default.go index 8bb7a506..be4a0f67 100644 --- a/Server/plugin/sandbox_default.go +++ b/Server/plugin/sandbox_default.go @@ -3,6 +3,7 @@ // Default plugin runtime: no Wazero. Plugin manifests are still discovered, // persisted, and surfaced through the admin API, but `.wasm` modules are not // executed. To enable real WASM execution build with `-tags wazero`. + package plugin import ( diff --git a/Server/ws/dm_handlers_test.go b/Server/ws/dm_handlers_test.go index c9204855..cdbf6153 100644 --- a/Server/ws/dm_handlers_test.go +++ b/Server/ws/dm_handlers_test.go @@ -294,7 +294,7 @@ func TestDM_ChatEdit_ParticipantCanEdit(t *testing.T) { hub.HandleMessageForTest(cAlice, dmChatEditMsg(msgID, "edited")) time.Sleep(100 * time.Millisecond) - // Alice should receive the chat_edited broadcast (via broadcastToDMParticipants). + // Alice should receive the chat_edited broadcast (via the sequenced DM event path). msgs := dmDrainAll(sendAlice) edited := dmFindMsgType(msgs, "chat_edited") if edited == nil { diff --git a/Server/ws/emit_test.go b/Server/ws/emit_test.go index 07ebacf4..a49087cb 100644 --- a/Server/ws/emit_test.go +++ b/Server/ws/emit_test.go @@ -186,7 +186,7 @@ func TestEmitEvents_ExcludeSenderEvent(t *testing.T) { h.EmitEvents(events) - // broadcastExclude is synchronous — check immediately. + // broadcastExcludeLow is synchronous — check immediately. senderMsgs := drainChan(sendSender, 50*time.Millisecond) otherMsgs := drainChan(sendOther, 50*time.Millisecond) diff --git a/Server/ws/event_pruner.go b/Server/ws/event_pruner.go index 2ce27647..f9b62574 100644 --- a/Server/ws/event_pruner.go +++ b/Server/ws/event_pruner.go @@ -3,6 +3,7 @@ // StartEventPruner runs a background goroutine that deletes events older than // the configured retention window. It is the bounded-storage half of the // event persistence design: the persister appends, the pruner trims. + package ws import ( diff --git a/Server/ws/handlers.go b/Server/ws/handlers.go index 5c18fb65..05160eca 100644 --- a/Server/ws/handlers.go +++ b/Server/ws/handlers.go @@ -5,30 +5,11 @@ import ( "encoding/json" "fmt" "log/slog" - "time" - "github.com/microcosm-cc/bluemonday" "github.com/owncord/server/auth" "github.com/owncord/server/db" ) -// Rate limit windows. -const ( - chatRateLimit = 10 - chatWindow = time.Second - typingRateLimit = 1 - typingWindow = 3 * time.Second - presenceRateLimit = 1 - presenceWindow = 10 * time.Second - reactionRateLimit = 5 - reactionWindow = time.Second -) - -// maxMessageLen is the maximum allowed message length in runes (Unicode code points). -const maxMessageLen = 4000 - -var sanitizer = bluemonday.StrictPolicy() - // HandleMessageForTest dispatches a raw WebSocket message from client c. // Exported so ws_test package can invoke it directly without a real connection. func (h *Hub) HandleMessageForTest(c *Client, raw []byte) { @@ -226,21 +207,11 @@ func (h *Hub) requireChannelPerm(c *Client, channelID int64, perm int64, permLab return false } -// broadcastExclude sends a message to all clients in the sender's channel -// EXCEPT the sender. Unlike hub.BroadcastToChannel, messages sent via this -// function are NOT stored in the replay ring buffer — they are ephemeral. -// This is correct for typing indicators but would be incorrect for messages -// that should survive reconnection replay. -func (h *Hub) broadcastExclude(channelID, excludeUserID int64, msg []byte) { - if channelID == 0 { - h.pubsub.Publish(TopicGlobal, msg, excludeUserID) - return - } - h.pubsub.Publish(ChannelTopic(channelID), msg, excludeUserID) -} - -// broadcastExcludeLow is like broadcastExclude but at low priority. -// Used for typing indicators — dropped on overflow instead of disconnecting. +// broadcastExcludeLow sends a message at low priority to all clients in the +// sender's channel EXCEPT the sender. Messages sent via this function are NOT +// stored in the replay ring buffer — they are ephemeral. This is correct for +// typing indicators (dropped on overflow instead of disconnecting) but would +// be incorrect for messages that should survive reconnection replay. func (h *Hub) broadcastExcludeLow(channelID, excludeUserID int64, msg []byte) { if channelID == 0 { h.pubsub.PublishLow(TopicGlobal, msg, excludeUserID) @@ -249,31 +220,3 @@ func (h *Hub) broadcastExcludeLow(channelID, excludeUserID int64, msg []byte) { h.pubsub.PublishLow(ChannelTopic(channelID), msg, excludeUserID) } -// broadcastToDMParticipants sends a message to all participants of a DM channel -// while preserving DM semantics (delivery is by participant, not channel focus). -// Unlike broadcastToDMParticipantsExclude, this path is sequenced and replayable. -func (h *Hub) broadcastToDMParticipants(channelID int64, msg []byte) { - participantIDs, err := h.db.GetDMParticipantIDs(channelID) - if err != nil { - slog.Error("broadcastToDMParticipants GetDMParticipantIDs", "err", err, "channel_id", channelID) - return - } - h.sendSequencedToUsers(channelID, participantIDs, msg) -} - -// broadcastToDMParticipantsExclude sends a message to all participants of a DM -// channel EXCEPT the specified user. Used for ephemeral events like typing -// indicators where echoing back to the sender is undesirable. -func (h *Hub) broadcastToDMParticipantsExclude(channelID, excludeUserID int64, msg []byte) { - participantIDs, err := h.db.GetDMParticipantIDs(channelID) - if err != nil { - slog.Error("broadcastToDMParticipantsExclude GetDMParticipantIDs", "err", err, "channel_id", channelID) - return - } - for _, pid := range participantIDs { - if pid == excludeUserID { - continue - } - h.SendToUser(pid, msg) - } -} diff --git a/Server/ws/handlers_command.go b/Server/ws/handlers_command.go index 1c3f8704..7600c6e7 100644 --- a/Server/ws/handlers_command.go +++ b/Server/ws/handlers_command.go @@ -5,6 +5,7 @@ // plugin returns a Reply, it is sent only to the invoking client (ephemeral). // If the plugin returns a Broadcast string, it is broadcast to the channel // only after verifying the invoking client holds SEND_MESSAGES permission. + package ws import ( diff --git a/Server/ws/handlers_test.go b/Server/ws/handlers_test.go index 215e102b..7db04d3d 100644 --- a/Server/ws/handlers_test.go +++ b/Server/ws/handlers_test.go @@ -1695,7 +1695,7 @@ func TestTyping_RateLimited_SilentlyDropped(t *testing.T) { // TestBroadcastExclude_SendsToOthersNotSelf verifies that broadcastExclude // delivers to all channel members except the excluded user. -// This is exercised indirectly via typing_start (which calls broadcastExclude). +// This is exercised indirectly via typing_start (which calls broadcastExcludeLow). func TestBroadcastExclude_SendsToOthersNotSelf(t *testing.T) { hub, database := newHandlerHub(t) chID := seedTestChannel(t, database, "excl-chan1") diff --git a/Server/ws/hub.go b/Server/ws/hub.go index 623ce27c..48deda38 100644 --- a/Server/ws/hub.go +++ b/Server/ws/hub.go @@ -555,26 +555,9 @@ func (h *Hub) BroadcastToAllLow(msg []byte) { h.pubsub.PublishGlobalLow(msg) } -// sendSequencedToUsers stamps msg with a monotonic seq, stores it in the replay -// buffer under channelID, and fanouts the wrapped payload to the provided users. -func (h *Hub) sendSequencedToUsers(channelID int64, userIDs []int64, msg []byte) { - h.seqMu.Lock() - defer h.seqMu.Unlock() - - seq := h.nextSeq() - wrapped := wrapWithSeq(msg, seq) - - // Store DM event for reconnect replay; filtering is channel-based and uses - // allowed channel IDs computed at auth time (including open DMs). - h.replayBuf.Push(seq, channelID, wrapped) - h.persistEvent(seq, channelID, wrapped) - - for _, userID := range userIDs { - h.SendToUser(userID, wrapped) - } -} - -// sendSequencedToUsersHigh is like sendSequencedToUsers but uses high-priority delivery. +// sendSequencedToUsersHigh stamps msg with a monotonic seq, stores it in the +// replay buffer under channelID, and fans the wrapped payload out to the +// provided users with high-priority delivery. func (h *Hub) sendSequencedToUsersHigh(channelID int64, userIDs []int64, msg []byte) { h.seqMu.Lock() defer h.seqMu.Unlock() diff --git a/Server/ws/pubsub.go b/Server/ws/pubsub.go index 7891d229..c86a9ff4 100644 --- a/Server/ws/pubsub.go +++ b/Server/ws/pubsub.go @@ -2,7 +2,6 @@ package ws import ( "fmt" - "log/slog" "sync" ) @@ -217,16 +216,3 @@ func (ps *PubSub) TopicsForClient(userID int64) []Topic { } return result } - -// debugDump logs the current subscription state. For development use only. -func (ps *PubSub) debugDump() { - ps.mu.RLock() - defer ps.mu.RUnlock() - for topic, subs := range ps.topics { - ids := make([]int64, 0, len(subs)) - for uid := range subs { - ids = append(ids, uid) - } - slog.Debug("pubsub: topic", "topic", string(topic), "subscribers", ids) - } -}