fix: address code review — 8 issues across server and client

Server fixes:
- voice_leave: broadcast voice_leave even on DB error so peers don't see ghost users (H1)
- migrate: record migration inside transaction for atomicity (H2)
- password: use init() with panic for dummyHash to catch bcrypt init failures (M1)
- password: replace //nolint:errcheck with explicit _ = discard (M2)
- handlers: clarify edit permission comment, fix error message wording (M3)
- auth_handler: distinguish duplicate username (400) from DB error (500) (M4)

Client fixes:
- media: revert YouTube oEmbed to browser fetch — no need to disable cert verification (C1)
- messages.store: fix prependMessages cap to keep newest messages, not oldest (H5)
- ChannelSidebar: ref-count globalDragAc to prevent multi-instance teardown race (H6)
- attachments: replace console.error with project logger (M6)
- ws: clarify lastSeq reset comment to match actual behavior (M5)

Client/tauri-client/src/components/ChannelSidebar.ts

@@ -385,10 +385,14 @@ function attachChannelContextMenu(
   );
 }
 
-/** Global mousemove/mouseup handlers for drag reordering. Registered once. */
+/** Global mousemove/mouseup handlers for drag reordering. Registered once.
+ *  Reference-counted so multiple sidebar instances share the same listeners
+ *  and only the last destroy tears them down. */
 let globalDragAc: AbortController | null = null;
+let globalDragRefCount = 0;
 
 function ensureGlobalDragListeners(): void {
+  globalDragRefCount++;
   if (globalDragAc !== null) {
     return;
   }
@@ -785,8 +789,11 @@ export function createChannelSidebar(options: ChannelSidebarOptions): MountableC
 
   function destroy(): void {
     ac.abort();
-    globalDragAc?.abort();
-    globalDragAc = null;
+    globalDragRefCount = Math.max(0, globalDragRefCount - 1);
+    if (globalDragRefCount === 0 && globalDragAc !== null) {
+      globalDragAc.abort();
+      globalDragAc = null;
+    }
     for (const unsub of unsubscribers) {
       unsub();
     }

Client/tauri-client/src/components/message-list/attachments.ts

@@ -11,8 +11,11 @@ import {
 import { createIcon } from "@lib/icons";
 import { observeMedia } from "@lib/media-visibility";
 import { loadPref } from "@components/settings/helpers";
+import { createLogger } from "@lib/logger";
 import { fetch as tauriFetch } from "@tauri-apps/plugin-http";
 import { save } from "@tauri-apps/plugin-dialog";
+
+const log = createLogger("attachments");
 import { writeFile } from "@tauri-apps/plugin-fs";
 import type { Attachment } from "@lib/types";
 import { openImageLightbox } from "./media";
@@ -175,7 +178,7 @@ export function fetchImageAsDataUrl(url: string): Promise<string | null> {
 
       return dataUrl;
     } catch (err) {
-      console.error("Failed to fetch attachment image:", url, err);
+      log.error("Failed to fetch attachment image", { url, error: String(err) });
       return null;
     }
   })();

Client/tauri-client/src/components/message-list/media.ts

@@ -12,7 +12,6 @@ import { createIcon } from "@lib/icons";
 import { createLogger } from "@lib/logger";
 import { observeMedia } from "@lib/media-visibility";
 import { loadPref } from "@components/settings/helpers";
-import { fetch as tauriFetch } from "@tauri-apps/plugin-http";
 import { isSafeUrl } from "./attachments";
 import { CODE_BLOCK_REGEX, INLINE_CODE_REGEX, URL_REGEX } from "./content-parser";
 import { renderGenericLinkPreview } from "./embeds";
@@ -121,10 +120,9 @@ export function renderYouTubeEmbed(videoId: string, originalUrl: string): HTMLDi
   } else {
     setText(titleLink, "Loading...");
     const oembedUrl = `https://www.youtube.com/oembed?url=https://www.youtube.com/watch?v=${encodeURIComponent(videoId)}&format=json`;
-    tauriFetch(oembedUrl, {
+    fetch(oembedUrl, {
       signal: AbortSignal.timeout(5000),
-      danger: { acceptInvalidCerts: true, acceptInvalidHostnames: false },
-    } as RequestInit)
+    })
       .then((res) => (res.ok ? (res.json() as Promise<{ title?: string } | null>) : null))
       .then((data) => {
         const title = data?.title ?? "YouTube Video";

Client/tauri-client/src/lib/ws.ts

@@ -370,8 +370,9 @@ export function createWsClient() {
     cleanupEventListeners();
     void disconnectProxy();
     setState("disconnected");
-    // Only reset lastSeq on intentional disconnect (e.g. logout)
-    // so reconnect scenarios preserve replay ability.
+    // Reset lastSeq — disconnect() is only called for intentional close
+    // (logout). Automatic reconnects go through scheduleReconnect() which
+    // preserves lastSeq for server-side event replay.
     lastSeq = 0;
   }
 

Client/tauri-client/src/stores/messages.store.ts

@@ -168,9 +168,9 @@ export function prependMessages(
   messagesStore.setState((prev) => {
     const existing = prev.messagesByChannel.get(channelId) ?? [];
     let combined = [...converted, ...existing];
-    // Keep oldest messages (start of array) since we're loading history
+    // Keep newest messages (end of array); drop oldest loaded history when cap exceeded
     if (combined.length > MAX_MESSAGES_PER_CHANNEL) {
-      combined = combined.slice(0, MAX_MESSAGES_PER_CHANNEL);
+      combined = combined.slice(combined.length - MAX_MESSAGES_PER_CHANNEL);
     }
     const updatedMessages = new Map(prev.messagesByChannel);
     updatedMessages.set(channelId, combined);

Server/api/auth_handler.go

@@ -128,10 +128,17 @@ func handleRegister(database *db.DB) http.HandlerFunc {
 		// Create user with default Member role.
 		uid, err := database.CreateUser(req.Username, hash, int(permissions.MemberRoleID))
 		if err != nil {
-			writeJSON(w, http.StatusBadRequest, errorResponse{
-				Error:   "INVALID_INPUT",
-				Message: "registration failed — check your details",
-			})
+			// UNIQUE constraint violation → duplicate username → 400.
+			// Any other DB error → 500.
+			if strings.Contains(err.Error(), "UNIQUE constraint") {
+				writeJSON(w, http.StatusBadRequest, genericAuthError)
+			} else {
+				slog.Error("CreateUser failed", "err", err, "username", req.Username)
+				writeJSON(w, http.StatusInternalServerError, errorResponse{
+					Error:   "SERVER_ERROR",
+					Message: "registration failed — please try again",
+				})
+			}
 			return
 		}
 

Server/auth/password.go

@@ -31,7 +31,15 @@ func HashPassword(password string) (string, error) {
 // when the user does not exist. Comparing against this dummy ensures that
 // CheckPassword takes roughly constant time regardless of whether a valid hash
 // was supplied.
-var dummyHash, _ = bcrypt.GenerateFromPassword([]byte("dummy-timing-pad"), bcryptCost)
+var dummyHash []byte
+
+func init() {
+	h, err := bcrypt.GenerateFromPassword([]byte("dummy-timing-pad"), bcryptCost)
+	if err != nil {
+		panic("auth: failed to generate dummy bcrypt hash: " + err.Error())
+	}
+	dummyHash = h
+}
 
 // CheckPassword reports whether password matches hash. Returns false on any
 // error, including an empty or malformed hash. When hash is empty (user does
@@ -41,7 +49,10 @@ func CheckPassword(hash, password string) bool {
 	if hash == "" {
 		// Perform a dummy comparison so the response time is indistinguishable
 		// from a real check, preventing timing-based username enumeration.
-		bcrypt.CompareHashAndPassword(dummyHash, []byte(password)) //nolint:errcheck
+		// The error is intentionally discarded: we always return false here.
+		// The comparison is performed only to consume time and prevent
+		// timing-based username enumeration.
+		_ = bcrypt.CompareHashAndPassword(dummyHash, []byte(password))
 		return false
 	}
 	err := bcrypt.CompareHashAndPassword([]byte(hash), []byte(password))

Server/db/migrate.go

@@ -175,22 +175,28 @@ func MigrateFS(database *DB, fsys fs.FS) error {
 
 		raw, readErr := fs.ReadFile(fsys, name)
 		if readErr != nil {
-			tx.Rollback() //nolint:errcheck
+			_ = tx.Rollback() // error ignored: already handling the triggering error
 			return fmt.Errorf("reading migration %s: %w", name, readErr)
 		}
 
 		if _, execErr := tx.Exec(string(raw)); execErr != nil {
-			tx.Rollback() //nolint:errcheck
+			_ = tx.Rollback() // error ignored: already handling the triggering error
 			return fmt.Errorf("executing migration %s: %w", name, execErr)
 		}
 
+		// Record the migration inside the same transaction so the migration
+		// and its tracking record are atomic. A crash between commit and
+		// record would otherwise cause re-application on next startup.
+		if _, execErr := tx.Exec(
+			"INSERT INTO schema_versions (version) VALUES (?)", name,
+		); execErr != nil {
+			_ = tx.Rollback()
+			return fmt.Errorf("recording migration %s: %w", name, execErr)
+		}
+
 		if commitErr := tx.Commit(); commitErr != nil {
 			return fmt.Errorf("commit migration %s: %w", name, commitErr)
 		}
-
-		if err := recordApplied(database, name); err != nil {
-			return err
-		}
 	}
 
 	return nil

Server/ws/handlers.go

@@ -310,8 +310,10 @@ func (h *Hub) handleChatEdit(c *Client, _ string, payload json.RawMessage) {
 	}
 
 	// Re-check that the user still has SendMessages permission on this channel.
+	// Editing an existing message requires the same permission as sending a new one —
+	// if a channel goes read-only, users cannot modify existing messages either.
 	if !h.hasChannelPerm(c, msg.ChannelID, permissions.SendMessages) {
-		c.sendMsg(buildErrorMsg(ErrCodeForbidden, "no send permission in this channel"))
+		c.sendMsg(buildErrorMsg(ErrCodeForbidden, "no permission to edit in this channel"))
 		return
 	}
 

Server/ws/voice_leave.go

@@ -19,7 +19,9 @@ func (h *Hub) handleVoiceLeave(c *Client) {
 		slog.Error("ws handleVoiceLeave LeaveVoiceChannel — ghost session may remain in DB",
 			"err", leaveErr, "user_id", c.userID, "channel_id", oldChID)
 		c.sendMsg(buildErrorMsg(ErrCodeInternal, "voice leave failed — please rejoin if issues persist"))
-		return
+		// Do NOT return: broadcast the leave so peers update their UI,
+		// even though the DB row may be stale. In-memory state (clearVoiceChID)
+		// was already cleared above.
 	}
 
 	h.BroadcastToAll(buildVoiceLeave(oldChID, c.userID))