From e0ab0744eee203fbd358d9c07e79f94288f4dab6 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 19 Jul 2026 17:47:39 +0000 Subject: [PATCH] feat(client): optimistic message send + composer permission gating Implements the two highest-impact gaps from the client UX spec. Optimistic send: - messages.store gains addOptimisticMessage / markSendFailed / removeOptimistic, and confirmSend now stamps the real id + "sent" on the ack. addMessage reconciles the broadcast by real id (idempotent, replay-safe) with a defensive author match, so an echo never duplicates. Message gains status/correlationId/errorCode. - ChannelController.performSend renders a pending row immediately and supports retry / delete-draft (retry preserves attachments). - MessageList renders pending (dimmed) and failed (reason + Retry / Delete) rows; the hover action bar is limited to confirmed rows. - Failures are precise: the server echoes the request id on error replies (buildErrorMsgWithID), so the dispatcher maps SLOW_MODE / FORBIDDEN / RATE_LIMITED / BAD_REQUEST to the exact row instead of dropping the code. An offline send is shown failed, not silently lost. Composer permission + connection gating: - The server computes an authoritative per-channel can_send in the ready payload (channelCanSend mirrors MessageService.checkSendPermission: READ|SEND, MANAGE_MESSAGES for announcement, admin bypass, channel overrides). channels.store carries it as Channel.canSend. - MessageInput gains a disabled-with-reason mode; ChannelController derives the reason from can_send + channel type + connection status and disables the composer reactively (announcement read-only, no-permission, reconnecting) rather than accepting a click and failing. Older servers that omit can_send default permissive. Docs: the corresponding "Current gap" callouts in docs/architecture/ux are updated to reflect the implementation. Verified: full server suite + client tsc + 3204 unit tests + lint + gofmt. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01UA17KPvqGBX3XbXYnMf1rA --- .../src/components/MessageInput.ts | 50 +++++- .../src/components/MessageList.ts | 4 + .../src/components/message-list/renderers.ts | 49 ++++- Client/tauri-client/src/lib/dispatcher.ts | 12 +- Client/tauri-client/src/lib/types.ts | 7 + .../src/pages/main-page/ChannelController.ts | 115 ++++++++++-- .../src/pages/main-page/SidebarDmHelpers.ts | 3 + .../tauri-client/src/stores/channels.store.ts | 8 + .../tauri-client/src/stores/messages.store.ts | 169 +++++++++++++++++- Client/tauri-client/src/styles/app.css | 20 +++ .../tests/unit/channel-controller.test.ts | 30 +++- .../tests/unit/channels.store.test.ts | 4 + .../tests/unit/dispatcher.test.ts | 42 ++++- .../tests/unit/message-list.test.ts | 3 + .../tests/unit/messages.store.test.ts | 108 +++++++++++ .../tests/unit/notifications.test.ts | 1 + .../tauri-client/tests/unit/renderers.test.ts | 45 +++++ .../tests/unit/screen-share-button.test.ts | 1 + .../tests/unit/sidebar-area.test.ts | 7 + .../tests/unit/sidebar-dm-helpers.test.ts | 4 + .../tests/unit/voice-widget.test.ts | 3 + Server/ws/can_send_ready_test.go | 46 +++++ Server/ws/can_send_test.go | 75 ++++++++ Server/ws/handlers.go | 8 +- Server/ws/messages.go | 18 ++ Server/ws/serve.go | 28 +++ docs/architecture/ux/README.md | 11 +- docs/architecture/ux/messaging.md | 71 ++++---- 28 files changed, 864 insertions(+), 78 deletions(-) create mode 100644 Server/ws/can_send_ready_test.go create mode 100644 Server/ws/can_send_test.go diff --git a/Client/tauri-client/src/components/MessageInput.ts b/Client/tauri-client/src/components/MessageInput.ts index cccab522..a55032d6 100644 --- a/Client/tauri-client/src/components/MessageInput.ts +++ b/Client/tauri-client/src/components/MessageInput.ts @@ -20,6 +20,8 @@ export interface MessageInputOptions { readonly onUploadFile?: (file: File) => Promise<{ id: string; url: string; filename: string }>; readonly onTyping: () => void; readonly onEditMessage: (messageId: number, content: string) => void; + /** Initial disabled reason (e.g. read-only / no-permission / offline). */ + readonly disabledReason?: string | null; } export type MessageInputComponent = MountableComponent & { @@ -27,6 +29,12 @@ export type MessageInputComponent = MountableComponent & { clearReply(): void; startEdit(messageId: number, content: string): void; cancelEdit(): void; + /** + * Disable the composer with a visible reason (permission / connection), or + * pass null to re-enable. Permission is expressed as affordance: a send that + * the server would refuse is prevented here, not attempted and rejected. + */ + setDisabled(reason: string | null): void; }; const TYPING_THROTTLE_MS = 3_000; @@ -59,6 +67,8 @@ export function createMessageInput(options: MessageInputOptions): MessageInputCo let replyBar: HTMLDivElement | null = null; let replyText: HTMLSpanElement | null = null; let editBar: HTMLDivElement | null = null; + let disabledReason: string | null = options.disabledReason ?? null; + const controlButtons: HTMLButtonElement[] = []; let attachmentPreviewBar: HTMLDivElement | null = null; /** Pending attachment IDs to send with the next message. */ @@ -128,7 +138,33 @@ export function createMessageInput(options: MessageInputOptions): MessageInputCo activeTimers.add(t); } + /** Reflect the current disabledReason onto the DOM (textarea + controls). */ + function applyDisabledState(): void { + if (textarea === null) return; + const disabled = disabledReason !== null; + textarea.disabled = disabled; + textarea.placeholder = disabled ? disabledReason! : `Message #${options.channelName}`; + for (const btn of controlButtons) { + if (disabled) { + btn.setAttribute("disabled", "true"); + } else { + // Don't re-enable the attach button when uploads aren't wired. + if (btn.classList.contains("attach-btn") && options.onUploadFile === undefined) continue; + btn.removeAttribute("disabled"); + } + } + if (root !== null) { + root.classList.toggle("composer-disabled", disabled); + } + } + + function setDisabled(reason: string | null): void { + disabledReason = reason; + applyDisabledState(); + } + function handleSend(): void { + if (disabledReason !== null) return; if (textarea === null) return; const content = textarea.value.trim(); const hasAttachments = pendingAttachments.length > 0; @@ -398,6 +434,12 @@ export function createMessageInput(options: MessageInputOptions): MessageInputCo "data-testid": "send-btn", }); sendBtn.appendChild(createIcon("send", 20)); + // Register interactive controls so the disabled state can toggle them. + controlButtons.length = 0; + controlButtons.push(sendBtn, emojiBtn, gifBtn); + if (options.onUploadFile !== undefined) { + controlButtons.push(attachBtn); + } textarea.addEventListener( "input", @@ -572,7 +614,11 @@ export function createMessageInput(options: MessageInputOptions): MessageInputCo appendChildren(inputBox, attachBtn, textarea, emojiBtn, gifBtn, sendBtn); appendChildren(root, replyBar, editBar, attachmentPreviewBar, inputBox); container.appendChild(root); - textarea.focus(); + // Apply any initial disabled reason before focusing. + applyDisabledState(); + if (disabledReason === null) { + textarea.focus(); + } } function destroy(): void { @@ -595,5 +641,5 @@ export function createMessageInput(options: MessageInputOptions): MessageInputCo attachmentPreviewBar = null; } - return { mount, destroy, setReplyTo, clearReply, startEdit, cancelEdit }; + return { mount, destroy, setReplyTo, clearReply, startEdit, cancelEdit, setDisabled }; } diff --git a/Client/tauri-client/src/components/MessageList.ts b/Client/tauri-client/src/components/MessageList.ts index 0dd363b4..63fd5584 100644 --- a/Client/tauri-client/src/components/MessageList.ts +++ b/Client/tauri-client/src/components/MessageList.ts @@ -27,6 +27,10 @@ export interface MessageListOptions { readonly onDeleteClick: (messageId: number) => void; readonly onReactionClick: (messageId: number, emoji: string) => void; readonly onPinClick: (messageId: number, channelId: number, currentlyPinned: boolean) => void; + /** Retry a failed optimistic send (by its correlation id). */ + readonly onRetry?: (correlationId: string) => void; + /** Discard a failed optimistic send without retrying. */ + readonly onDeleteDraft?: (correlationId: string) => void; } // -- Constants ---------------------------------------------------------------- diff --git a/Client/tauri-client/src/components/message-list/renderers.ts b/Client/tauri-client/src/components/message-list/renderers.ts index ef0276ad..acfdc195 100644 --- a/Client/tauri-client/src/components/message-list/renderers.ts +++ b/Client/tauri-client/src/components/message-list/renderers.ts @@ -129,6 +129,24 @@ function renderSystemMessage(msg: Message): HTMLDivElement { return el; } +/** Map a send-failure error code to a short, user-facing reason. */ +function sendErrorReason(code: string | null): string { + switch (code) { + case "SLOW_MODE": + return "Slow mode — wait before sending again"; + case "RATE_LIMITED": + return "You're sending too fast — try again in a moment"; + case "FORBIDDEN": + return "You don't have permission to post here"; + case "OFFLINE": + return "Not connected — message not sent"; + case "BAD_REQUEST": + return "Message rejected"; + default: + return "Failed to send"; + } +} + export function renderMessage( msg: Message, isGrouped: boolean, @@ -140,8 +158,10 @@ export function renderMessage( return renderSystemMessage(msg); } + const statusClass = + msg.status === "pending" ? " pending" : msg.status === "failed" ? " failed" : ""; const el = createElement("div", { - class: isGrouped ? "message grouped" : "message", + class: (isGrouped ? "message grouped" : "message") + statusClass, "data-testid": `message-${msg.id}`, }); @@ -217,7 +237,32 @@ export function renderMessage( } } - if (!msg.deleted) { + // Failed optimistic send: show the reason and offer retry / discard. + if (msg.status === "failed" && msg.correlationId !== null) { + const cid = msg.correlationId; + const bar = createElement("div", { class: "msg-send-failed" }); + bar.appendChild( + createElement("span", { class: "msg-send-failed-text" }, sendErrorReason(msg.errorCode)), + ); + const retryBtn = createElement( + "button", + { class: "msg-send-retry", "data-testid": `msg-retry-${cid}` }, + "Retry", + ); + retryBtn.addEventListener("click", () => opts.onRetry?.(cid), { signal }); + const discardBtn = createElement( + "button", + { class: "msg-send-discard", "data-testid": `msg-discard-${cid}` }, + "Delete", + ); + discardBtn.addEventListener("click", () => opts.onDeleteDraft?.(cid), { signal }); + appendChildren(bar, retryBtn, discardBtn); + el.appendChild(bar); + } + + // The hover action bar (react/reply/pin/edit/delete) only applies to + // confirmed server messages — not deleted rows or unsent optimistic rows. + if (!msg.deleted && msg.status === "sent") { const actionsBar = createElement("div", { class: "msg-actions-bar" }); const reactBtn = createElement("button", { diff --git a/Client/tauri-client/src/lib/dispatcher.ts b/Client/tauri-client/src/lib/dispatcher.ts index 1aac2358..8e636f23 100644 --- a/Client/tauri-client/src/lib/dispatcher.ts +++ b/Client/tauri-client/src/lib/dispatcher.ts @@ -21,6 +21,8 @@ import { deleteMessage, updateReaction, confirmSend, + markSendFailed, + messagesStore, } from "@stores/messages.store"; import { setMembers, @@ -419,10 +421,11 @@ export function wireDispatcher(ws: WsClient): DispatcherCleanup { ); unsubs.push( - ws.on(S.ERROR, (payload) => { + ws.on(S.ERROR, (payload, id) => { log.error("Server error", { code: payload.code, message: payload.message, + id, }); if (payload.code === "BANNED") { // Banned users must not reconnect — show error and force logout. @@ -430,6 +433,13 @@ export function wireDispatcher(ws: WsClient): DispatcherCleanup { clearAuth(); return; } + // If the error carries the request id of a pending optimistic send, mark + // that specific row failed (with retry) instead of a global toast. This + // covers SLOW_MODE, FORBIDDEN, RATE_LIMITED, BAD_REQUEST, etc. on send. + if (id && messagesStore.getState().pendingSends.has(id)) { + markSendFailed(id, payload.code); + return; + } if (payload.code === "RATE_LIMITED" || payload.code === "FORBIDDEN") { setTransientError(payload.message || "Server error"); } diff --git a/Client/tauri-client/src/lib/types.ts b/Client/tauri-client/src/lib/types.ts index d2db6df1..3f32ab2f 100644 --- a/Client/tauri-client/src/lib/types.ts +++ b/Client/tauri-client/src/lib/types.ts @@ -92,6 +92,13 @@ export interface ReadyChannel { readonly position: number; readonly unread_count?: number; readonly last_message_id?: number; + /** + * Whether the current user may post in this channel — authoritative, + * server-computed (base role ± channel overrides, admin bypass, and the + * announcement MANAGE_MESSAGES rule). Drives the composer affordance; the + * server still enforces. Absent from older servers. + */ + readonly can_send?: boolean; } /** Member object in the ready payload. */ diff --git a/Client/tauri-client/src/pages/main-page/ChannelController.ts b/Client/tauri-client/src/pages/main-page/ChannelController.ts index e2799d23..5e94b962 100644 --- a/Client/tauri-client/src/pages/main-page/ChannelController.ts +++ b/Client/tauri-client/src/pages/main-page/ChannelController.ts @@ -15,7 +15,15 @@ import type { MessageListComponent } from "@components/MessageList"; import { createMessageInput } from "@components/MessageInput"; import type { MessageInputComponent } from "@components/MessageInput"; import { createTypingIndicator } from "@components/TypingIndicator"; -import { getChannelMessages, setMessagePinned } from "@stores/messages.store"; +import { + getChannelMessages, + setMessagePinned, + addOptimisticMessage, + markSendFailed, + removeOptimistic, +} from "@stores/messages.store"; +import { authStore } from "@stores/auth.store"; +import type { MessageUser } from "@lib/types"; import type { MessageController } from "./MessageController"; import type { PendingDeleteManager } from "./MessageController"; import type { ReactionController } from "./ReactionController"; @@ -23,6 +31,7 @@ import { updateChatHeaderForDm } from "./ChatHeader"; import type { ChatHeaderRefs } from "./ChatHeader"; import { dmStore } from "@stores/dm.store"; import { membersStore } from "@stores/members.store"; +import { channelsStore } from "@stores/channels.store"; const log = createLogger("channel-ctrl"); @@ -83,10 +92,15 @@ export function createChannelController(opts: ChannelControllerOptions): Channel let messageList: MessageListComponent | null = null; let messageInput: MessageInputComponent | null = null; let typingIndicator: MountableComponent | null = null; + // Store/ws subscriptions that keep the composer's disabled state in sync. + let composerGatingUnsubs: (() => void)[] = []; function destroyChannel(): void { pendingDeleteManager.cleanup(); + for (const unsub of composerGatingUnsubs) unsub(); + composerGatingUnsubs = []; + if (channelAbort !== null) { channelAbort.abort(); channelAbort = null; @@ -128,6 +142,59 @@ export function createChannelController(opts: ChannelControllerOptions): Channel const signal = channelAbort.signal; const userId = getCurrentUserId(); + // Optimistic send: keep the raw payload per correlation id so a failed + // send can be retried (including its attachments). Channel-scoped — cleared + // when the channel unmounts. + const draftByCorrelation = new Map< + string, + { content: string; replyTo: number | null; attachments: readonly string[] } + >(); + + function currentMessageUser(): MessageUser | null { + const u = authStore.getState().user; + if (u === null) return null; + return { id: u.id, username: u.username, avatar: u.avatar }; + } + + function performSend( + content: string, + replyTo: number | null, + attachments: readonly string[], + ): void { + const user = currentMessageUser(); + if (user === null) return; + const timestamp = new Date().toISOString(); + if (ws.getState() !== "connected") { + // Composer gating normally prevents this, but stay consistent: show a + // failed row with retry rather than silently dropping the message. + const cid = crypto.randomUUID(); + addOptimisticMessage({ correlationId: cid, channelId, user, content, replyTo, timestamp }); + draftByCorrelation.set(cid, { content, replyTo, attachments }); + markSendFailed(cid, "OFFLINE"); + return; + } + const cid = ws.send({ + type: "chat_send", + payload: { channel_id: channelId, content, reply_to: replyTo, attachments }, + }); + addOptimisticMessage({ correlationId: cid, channelId, user, content, replyTo, timestamp }); + draftByCorrelation.set(cid, { content, replyTo, attachments }); + } + + function retrySend(correlationId: string): void { + const draft = draftByCorrelation.get(correlationId); + draftByCorrelation.delete(correlationId); + removeOptimistic(correlationId); + if (draft !== undefined) { + performSend(draft.content, draft.replyTo, draft.attachments); + } + } + + function deleteDraft(correlationId: string): void { + draftByCorrelation.delete(correlationId); + removeOptimistic(correlationId); + } + void msgCtrl.loadMessages(channelId, signal); // MessageList @@ -182,6 +249,8 @@ export function createChannelController(opts: ChannelControllerOptions): Channel showToast("Failed to pin/unpin message", "error"); }); }, + onRetry: (correlationId: string) => retrySend(correlationId), + onDeleteDraft: (correlationId: string) => deleteDraft(correlationId), }); messageList.mount(slots.messagesSlot); @@ -197,20 +266,7 @@ export function createChannelController(opts: ChannelControllerOptions): Channel channelId, channelName, onSend: (content: string, replyTo: number | null, attachments: readonly string[]) => { - if (ws.getState() !== "connected") { - log.warn("Cannot send message: not connected"); - showToast("Not connected — message not sent", "error"); - return; - } - ws.send({ - type: "chat_send", - payload: { - channel_id: channelId, - content, - reply_to: replyTo, - attachments, - }, - }); + performSend(content, replyTo, attachments); }, onUploadFile: async (file: File) => { try { @@ -250,6 +306,35 @@ export function createChannelController(opts: ChannelControllerOptions): Channel }); messageInput.mount(slots.inputSlot); + // Composer gating: express permission + connection as affordance. The + // composer disables (with a reason) when the socket is down or the user + // may not post here, instead of accepting a click and failing. DM channels + // are not in channelsStore (dmStore), so they are left ungated here — the + // server still enforces block/permission and a refused send shows as a + // failed row. + const computeComposerReason = (): string | null => { + if (ws.getState() !== "connected") return "Reconnecting…"; + const ch = channelsStore.getState().channels.get(channelId); + if (ch === undefined) return null; + if (!ch.canSend) { + return ch.type === "announcement" + ? "Only moderators can post in announcement channels" + : "You don't have permission to send messages here"; + } + return null; + }; + const refreshComposerState = (): void => { + messageInput?.setDisabled(computeComposerReason()); + }; + refreshComposerState(); + composerGatingUnsubs.push(ws.onStateChange(() => refreshComposerState())); + composerGatingUnsubs.push( + channelsStore.subscribeSelector( + (s) => s.channels.get(channelId)?.canSend ?? true, + () => refreshComposerState(), + ), + ); + // Arrow-up edit: listen for edit-last-message bubbling from MessageInput slots.inputSlot.addEventListener( "edit-last-message", diff --git a/Client/tauri-client/src/pages/main-page/SidebarDmHelpers.ts b/Client/tauri-client/src/pages/main-page/SidebarDmHelpers.ts index 545ba8f1..bb88f57d 100644 --- a/Client/tauri-client/src/pages/main-page/SidebarDmHelpers.ts +++ b/Client/tauri-client/src/pages/main-page/SidebarDmHelpers.ts @@ -72,6 +72,9 @@ export function addDmToChannelsStore(dmChannel: DmChannel): void { position: 0, unreadCount: dmChannel.unreadCount, lastMessageId: dmChannel.lastMessageId, + // DMs are always postable from the client; the server enforces block state, + // and a refused send surfaces as a failed row rather than a disabled composer. + canSend: true, }; channelsStore.setState((prev) => { const next = new Map(prev.channels); diff --git a/Client/tauri-client/src/stores/channels.store.ts b/Client/tauri-client/src/stores/channels.store.ts index 4b8588bb..3135e922 100644 --- a/Client/tauri-client/src/stores/channels.store.ts +++ b/Client/tauri-client/src/stores/channels.store.ts @@ -20,6 +20,8 @@ export interface Channel { readonly position: number; readonly unreadCount: number; readonly lastMessageId: number | null; + /** Whether the current user may post here (drives the composer affordance). */ + readonly canSend: boolean; } export interface ChannelsState { @@ -48,6 +50,9 @@ export function setChannels(channels: readonly ReadyChannel[]): void { position: ch.position, unreadCount: ch.unread_count ?? 0, lastMessageId: ch.last_message_id ?? null, + // The current server always sends can_send; older servers omit it, in + // which case we default permissive (no gating) rather than guessing. + canSend: ch.can_send ?? true, }); } channelsStore.setState((prev) => ({ @@ -80,6 +85,9 @@ export function addChannel(channel: ChannelCreatePayload): void { position: channel.position, unreadCount: 0, lastMessageId: null, + // Broadcasts carry no per-user data; default permissive. The next ready + // payload delivers the authoritative can_send. Server enforces regardless. + canSend: true, }); return { ...prev, channels: next }; }); diff --git a/Client/tauri-client/src/stores/messages.store.ts b/Client/tauri-client/src/stores/messages.store.ts index 8ecae7a2..a55ab9aa 100644 --- a/Client/tauri-client/src/stores/messages.store.ts +++ b/Client/tauri-client/src/stores/messages.store.ts @@ -20,6 +20,14 @@ import type { // Types // ----------------------------------------------------------------------------- +/** + * Delivery status of a message row. + * - "sent": confirmed by the server (the default for every server-sourced row). + * - "pending": optimistic local row awaiting the chat_send_ok ack. + * - "failed": the send was rejected or dropped; the row offers retry. + */ +export type MessageStatus = "sent" | "pending" | "failed"; + export interface Message { readonly id: number; readonly channelId: number; @@ -32,6 +40,15 @@ export interface Message { readonly editedAt: string | null; readonly deleted: boolean; readonly timestamp: string; + /** Delivery status. Server-sourced rows are always "sent". */ + readonly status: MessageStatus; + /** + * Correlation id for an optimistic row, matching the id echoed on + * chat_send_ok / error. Null once reconciled or for server-sourced rows. + */ + readonly correlationId: string | null; + /** Error code when status === "failed" (e.g. "SLOW_MODE", "FORBIDDEN"). */ + readonly errorCode: string | null; } export interface MessagesState { @@ -62,6 +79,9 @@ function chatPayloadToMessage(payload: ChatMessagePayload): Message { editedAt: null, deleted: false, timestamp: payload.timestamp, + status: "sent", + correlationId: null, + errorCode: null, }; } @@ -78,6 +98,9 @@ function messageResponseToMessage(response: MessageResponse): Message { editedAt: response.edited_at, deleted: response.deleted, timestamp: response.timestamp, + status: "sent", + correlationId: null, + errorCode: null, }; } @@ -105,12 +128,47 @@ export const messagesStore = createStore(INITIAL_STATE); // Actions // ----------------------------------------------------------------------------- -/** Append a new message from a chat_message WS event. */ +/** + * Append a new message from a chat_message WS event, reconciling with any + * optimistic row it corresponds to. + * + * Reconciliation (the server sends chat_send_ok before the broadcast, so by the + * time our own echo arrives the optimistic row already carries its real id): + * 1. If a row with the same real id exists, replace it in place — this turns + * an optimistic "sent" row into the full server message (attachments, + * sanitized content, server timestamp) and is idempotent against replay. + * 2. Otherwise, defensively reconcile the oldest still-pending row from the + * same author (covers a broadcast that raced ahead of its ack). + * 3. Otherwise, append as a new message. + */ export function addMessage(payload: ChatMessagePayload): void { const message = chatPayloadToMessage(payload); messagesStore.setState((prev) => { const channelId = message.channelId; const existing = prev.messagesByChannel.get(channelId) ?? []; + + // 1. Replace an existing row with the same real id (reconcile / idempotent). + const idIdx = existing.findIndex((m) => m.id !== 0 && m.id === message.id); + if (idIdx !== -1) { + const replaced = existing.map((m, i) => (i === idIdx ? message : m)); + const updated = new Map(prev.messagesByChannel); + updated.set(channelId, replaced); + return { ...prev, messagesByChannel: updated }; + } + + // 2. Defensive: reconcile the oldest pending optimistic row from this author + // (a broadcast that arrived before its chat_send_ok ack). + const pendingIdx = existing.findIndex( + (m) => m.status === "pending" && m.correlationId !== null && m.user.id === message.user.id, + ); + if (pendingIdx !== -1) { + const replaced = existing.map((m, i) => (i === pendingIdx ? message : m)); + const updated = new Map(prev.messagesByChannel); + updated.set(channelId, replaced); + return { ...prev, messagesByChannel: updated }; + } + + // 3. Append as a new message. let updatedMsgs = [...existing, message]; // Evict oldest messages if over the cap if (updatedMsgs.length > MAX_MESSAGES_PER_CHANNEL) { @@ -127,6 +185,84 @@ export function addMessage(payload: ChatMessagePayload): void { }); } +/** + * Insert an optimistic pending row for a message the user just sent. The row + * carries the correlationId returned by ws.send and renders immediately as + * "sending"; confirmSend / markSendFailed reconcile it against the server. + */ +export function addOptimisticMessage(params: { + correlationId: string; + channelId: number; + user: MessageUser; + content: string; + replyTo: number | null; + attachments?: readonly Attachment[]; + timestamp: string; +}): void { + const optimistic: Message = { + id: 0, + channelId: params.channelId, + user: params.user, + content: params.content, + replyTo: params.replyTo, + attachments: params.attachments ?? [], + reactions: [], + pinned: false, + editedAt: null, + deleted: false, + timestamp: params.timestamp, + status: "pending", + correlationId: params.correlationId, + errorCode: null, + }; + messagesStore.setState((prev) => { + const existing = prev.messagesByChannel.get(params.channelId) ?? []; + const updated = new Map(prev.messagesByChannel); + updated.set(params.channelId, [...existing, optimistic]); + const updatedPending = new Map(prev.pendingSends); + updatedPending.set(params.correlationId, params.channelId); + return { ...prev, messagesByChannel: updated, pendingSends: updatedPending }; + }); +} + +/** Mark an optimistic row as failed so the UI can offer retry. */ +export function markSendFailed(correlationId: string, errorCode: string | null): void { + messagesStore.setState((prev) => { + const channelId = prev.pendingSends.get(correlationId); + if (channelId === undefined) return prev; + const existing = prev.messagesByChannel.get(channelId); + if (existing === undefined) return prev; + const updatedList = existing.map((m) => + m.correlationId === correlationId ? { ...m, status: "failed" as const, errorCode } : m, + ); + const updatedMessages = new Map(prev.messagesByChannel); + updatedMessages.set(channelId, updatedList); + const updatedPending = new Map(prev.pendingSends); + updatedPending.delete(correlationId); + return { ...prev, messagesByChannel: updatedMessages, pendingSends: updatedPending }; + }); +} + +/** Remove an optimistic row (retry discards the old row; delete-draft dismisses it). */ +export function removeOptimistic(correlationId: string): void { + messagesStore.setState((prev) => { + const channelId = prev.pendingSends.get(correlationId); + const updatedPending = new Map(prev.pendingSends); + updatedPending.delete(correlationId); + if (channelId === undefined) { + return { ...prev, pendingSends: updatedPending }; + } + const existing = prev.messagesByChannel.get(channelId); + if (existing === undefined) { + return { ...prev, pendingSends: updatedPending }; + } + const filtered = existing.filter((m) => m.correlationId !== correlationId); + const updatedMessages = new Map(prev.messagesByChannel); + updatedMessages.set(channelId, filtered); + return { ...prev, messagesByChannel: updatedMessages, pendingSends: updatedPending }; + }); +} + /** Bulk set messages from a REST response. Marks channel as loaded. * The server returns messages newest-first; we reverse to chronological order. */ export function setMessages( @@ -248,12 +384,33 @@ export function addPendingSend(correlationId: string, channelId: number): void { }); } -/** Confirm a pending send — remove from pending map. */ -export function confirmSend(correlationId: string, _messageId: number, _timestamp: string): void { +/** + * Confirm a pending send from a chat_send_ok ack: stamp the optimistic row with + * its real server id + timestamp and mark it "sent". The subsequent + * chat_message broadcast then reconciles by real id (addMessage step 1), + * upgrading the row to the full server message. Removing it from pendingSends + * makes a late error a no-op. + */ +export function confirmSend(correlationId: string, messageId: number, timestamp: string): void { messagesStore.setState((prev) => { - const updated = new Map(prev.pendingSends); - updated.delete(correlationId); - return { ...prev, pendingSends: updated }; + const channelId = prev.pendingSends.get(correlationId); + const updatedPending = new Map(prev.pendingSends); + updatedPending.delete(correlationId); + if (channelId === undefined) { + return { ...prev, pendingSends: updatedPending }; + } + const existing = prev.messagesByChannel.get(channelId); + if (existing === undefined) { + return { ...prev, pendingSends: updatedPending }; + } + const updatedList = existing.map((m) => + m.correlationId === correlationId + ? { ...m, id: messageId, timestamp, status: "sent" as const, errorCode: null } + : m, + ); + const updatedMessages = new Map(prev.messagesByChannel); + updatedMessages.set(channelId, updatedList); + return { ...prev, messagesByChannel: updatedMessages, pendingSends: updatedPending }; }); } diff --git a/Client/tauri-client/src/styles/app.css b/Client/tauri-client/src/styles/app.css index f35e55f0..91d5c2cd 100644 --- a/Client/tauri-client/src/styles/app.css +++ b/Client/tauri-client/src/styles/app.css @@ -896,6 +896,26 @@ min-height: 36px; max-height: 200px; line-height: 1.4; } .msg-textarea::placeholder { color: var(--text-micro); } +.msg-textarea:disabled { cursor: not-allowed; opacity: 0.7; } +/* Composer disabled (offline / read-only / no permission): dim the whole bar. */ +.message-input-wrap.composer-disabled .message-input-box { opacity: 0.6; } +.message-input-wrap.composer-disabled .input-btn { cursor: not-allowed; } + +/* ── Optimistic send states ── */ +/* Pending: the row is dimmed until the server confirms it. */ +.message.pending { opacity: 0.6; } +/* Failed: tint + an inline retry/discard affordance. */ +.message.failed { opacity: 0.9; } +.msg-send-failed { + display: flex; align-items: center; gap: 8px; + margin-top: 4px; font-size: 12px; color: var(--danger, #e57373); +} +.msg-send-failed-text { flex-shrink: 0; } +.msg-send-retry, .msg-send-discard { + background: transparent; border: 1px solid currentColor; color: inherit; + border-radius: 4px; padding: 1px 8px; font-size: 12px; cursor: pointer; +} +.msg-send-retry:hover, .msg-send-discard:hover { background: rgba(229, 115, 115, 0.15); } /* ── Member List ── */ .member-list { diff --git a/Client/tauri-client/tests/unit/channel-controller.test.ts b/Client/tauri-client/tests/unit/channel-controller.test.ts index 4ebfa789..103ad6a7 100644 --- a/Client/tauri-client/tests/unit/channel-controller.test.ts +++ b/Client/tauri-client/tests/unit/channel-controller.test.ts @@ -78,6 +78,9 @@ vi.mock("@components/MessageInput", () => ({ destroy: mockMessageInputDestroy, setReplyTo: mockSetReplyTo, startEdit: mockStartEdit, + clearReply: vi.fn(), + cancelEdit: vi.fn(), + setDisabled: vi.fn(), }; }), })); @@ -89,13 +92,26 @@ vi.mock("@components/TypingIndicator", () => ({ })), })); -const { mockSetMessagePinned } = vi.hoisted(() => ({ - mockSetMessagePinned: vi.fn(), -})); +const { mockSetMessagePinned, mockAddOptimistic, mockMarkSendFailed, mockRemoveOptimistic } = + vi.hoisted(() => ({ + mockSetMessagePinned: vi.fn(), + mockAddOptimistic: vi.fn(), + mockMarkSendFailed: vi.fn(), + mockRemoveOptimistic: vi.fn(), + })); vi.mock("@stores/messages.store", () => ({ getChannelMessages: mockGetChannelMessages, setMessagePinned: mockSetMessagePinned, + addOptimisticMessage: mockAddOptimistic, + markSendFailed: mockMarkSendFailed, + removeOptimistic: mockRemoveOptimistic, +})); + +vi.mock("@stores/auth.store", () => ({ + authStore: { + getState: () => ({ user: { id: 1, username: "tester", avatar: null } }), + }, })); const { mockUpdateChatHeaderForDm } = vi.hoisted(() => ({ @@ -152,6 +168,7 @@ function makeOpts(overrides: Partial = {}): ChannelCon ws: { send: vi.fn(), getState: vi.fn(() => "connected"), + onStateChange: vi.fn(() => vi.fn()), } as unknown as ChannelControllerOptions["ws"], api: { uploadFile: vi.fn().mockResolvedValue({ id: 1, url: "/f/1", filename: "f.txt" }), @@ -331,7 +348,7 @@ describe("createChannelController", () => { }); }); - it("onSend shows error when not connected", () => { + it("onSend while disconnected records a failed optimistic row (no silent drop)", () => { const opts = makeOpts(); (opts.ws.getState as ReturnType).mockReturnValue("disconnected"); const ctrl = createChannelController(opts); @@ -339,7 +356,10 @@ describe("createChannelController", () => { capturedMessageInputOpts.onSend("hello", null, []); - expect(opts.showToast).toHaveBeenCalledWith("Not connected — message not sent", "error"); + // No socket send is attempted; the row is shown as failed with retry. + expect(opts.ws.send).not.toHaveBeenCalledWith(expect.objectContaining({ type: "chat_send" })); + expect(mockAddOptimistic).toHaveBeenCalled(); + expect(mockMarkSendFailed).toHaveBeenCalledWith(expect.any(String), "OFFLINE"); }); it("onTyping sends typing_start via ws", () => { diff --git a/Client/tauri-client/tests/unit/channels.store.test.ts b/Client/tauri-client/tests/unit/channels.store.test.ts index 7614ffcf..cba6e2a0 100644 --- a/Client/tauri-client/tests/unit/channels.store.test.ts +++ b/Client/tauri-client/tests/unit/channels.store.test.ts @@ -73,6 +73,7 @@ describe("channels store", () => { position: 0, unreadCount: 3, lastMessageId: 100, + canSend: true, }); const voice = state.channels.get(2); @@ -84,6 +85,7 @@ describe("channels store", () => { position: 0, unreadCount: 0, lastMessageId: null, + canSend: true, }); }); @@ -120,6 +122,7 @@ describe("channels store", () => { position: 2, unreadCount: 0, lastMessageId: null, + canSend: true, }); }); @@ -282,6 +285,7 @@ describe("channels store", () => { position: 0, unreadCount: 0, lastMessageId: 100, + canSend: true, }); }); diff --git a/Client/tauri-client/tests/unit/dispatcher.test.ts b/Client/tauri-client/tests/unit/dispatcher.test.ts index 3e90ff71..1a7ed135 100644 --- a/Client/tauri-client/tests/unit/dispatcher.test.ts +++ b/Client/tauri-client/tests/unit/dispatcher.test.ts @@ -2,7 +2,11 @@ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; import { wireDispatcher } from "../../src/lib/dispatcher"; import { authStore, clearAuth } from "../../src/stores/auth.store"; import { channelsStore } from "../../src/stores/channels.store"; -import { messagesStore } from "../../src/stores/messages.store"; +import { + messagesStore, + addOptimisticMessage, + getChannelMessages, +} from "../../src/stores/messages.store"; import { membersStore } from "../../src/stores/members.store"; import { voiceStore } from "../../src/stores/voice.store"; import { dmStore } from "../../src/stores/dm.store"; @@ -188,6 +192,7 @@ describe("WS Dispatcher", () => { position: 0, unreadCount: 0, lastMessageId: null, + canSend: true, }); return { ...prev, channels: ch, activeChannelId: 1 }; // active is channel 1 }); @@ -247,6 +252,7 @@ describe("WS Dispatcher", () => { position: 0, unreadCount: 0, lastMessageId: null, + canSend: true, }); return { ...prev, channels: ch }; }); @@ -499,6 +505,7 @@ describe("WS Dispatcher", () => { position: 0, unreadCount: 0, lastMessageId: null, + canSend: true, }); return { ...prev, channels: ch }; }); @@ -526,6 +533,7 @@ describe("WS Dispatcher", () => { position: 0, unreadCount: 0, lastMessageId: null, + canSend: true, }); ch.set(20, { id: 20, @@ -535,6 +543,7 @@ describe("WS Dispatcher", () => { position: 1, unreadCount: 0, lastMessageId: null, + canSend: true, }); return { ...prev, channels: ch, activeChannelId: 10 }; }); @@ -556,6 +565,7 @@ describe("WS Dispatcher", () => { position: 0, unreadCount: 0, lastMessageId: null, + canSend: true, }); return { ...prev, channels: ch, activeChannelId: 10 }; }); @@ -782,6 +792,34 @@ describe("WS Dispatcher", () => { expect(uiStore.getState().transientError).toBeNull(); }); + it("wires an error carrying a pending send id to mark that row failed (not a toast)", () => { + messagesStore.setState(() => ({ + messagesByChannel: new Map(), + pendingSends: new Map(), + loadedChannels: new Set(), + hasMore: new Map(), + })); + uiStore.setState((prev) => ({ ...prev, transientError: null })); + + addOptimisticMessage({ + correlationId: "corr-1", + channelId: 7, + user: { id: 1, username: "alex", avatar: null }, + content: "hi", + replyTo: null, + timestamp: "2026-03-15T10:00:00Z", + }); + + // Error echoes the request id → the specific row is marked failed… + mock.dispatch("error", { code: "SLOW_MODE", message: "slow down" }, "corr-1"); + + const row = getChannelMessages(7)[0]!; + expect(row.status).toBe("failed"); + expect(row.errorCode).toBe("SLOW_MODE"); + // …and it is not surfaced as a global transient error. + expect(uiStore.getState().transientError).toBeNull(); + }); + it("does not increment unread for own messages", () => { authStore.setState((prev) => ({ ...prev, @@ -798,6 +836,7 @@ describe("WS Dispatcher", () => { position: 0, unreadCount: 0, lastMessageId: null, + canSend: true, }); return { ...prev, channels: ch, activeChannelId: 1 }; }); @@ -828,6 +867,7 @@ describe("WS Dispatcher", () => { position: 0, unreadCount: 0, lastMessageId: null, + canSend: true, }); return { ...prev, channels: ch, activeChannelId: 1 }; }); diff --git a/Client/tauri-client/tests/unit/message-list.test.ts b/Client/tauri-client/tests/unit/message-list.test.ts index aebd0434..69506b59 100644 --- a/Client/tauri-client/tests/unit/message-list.test.ts +++ b/Client/tauri-client/tests/unit/message-list.test.ts @@ -46,6 +46,9 @@ function makeMessage(overrides: Partial & { id: number }): Message { editedAt: null, deleted: false, timestamp: "2024-01-15T12:00:00Z", + status: "sent", + correlationId: null, + errorCode: null, ...overrides, }; } diff --git a/Client/tauri-client/tests/unit/messages.store.test.ts b/Client/tauri-client/tests/unit/messages.store.test.ts index e0441e38..7937cf5d 100644 --- a/Client/tauri-client/tests/unit/messages.store.test.ts +++ b/Client/tauri-client/tests/unit/messages.store.test.ts @@ -10,6 +10,9 @@ import { updateReaction, addPendingSend, confirmSend, + addOptimisticMessage, + markSendFailed, + removeOptimistic, getChannelMessages, isChannelLoaded, hasMoreMessages, @@ -871,4 +874,109 @@ describe("messages store", () => { expect(msg.deleted).toBe(false); }); }); + + // ------------------------------------------------------------------------- + // Optimistic send lifecycle + // ------------------------------------------------------------------------- + + describe("optimistic send", () => { + it("addOptimisticMessage inserts a pending row and tracks the correlation id", () => { + addOptimisticMessage({ + correlationId: "c1", + channelId: 1, + user: TEST_USER, + content: "hi", + replyTo: null, + timestamp: "2026-03-15T10:00:00Z", + }); + const msgs = getChannelMessages(1); + expect(msgs).toHaveLength(1); + expect(msgs[0]!.status).toBe("pending"); + expect(msgs[0]!.correlationId).toBe("c1"); + expect(msgs[0]!.id).toBe(0); + expect(messagesStore.getState().pendingSends.get("c1")).toBe(1); + }); + + it("confirmSend then the broadcast reconciles into a single sent message", () => { + addOptimisticMessage({ + correlationId: "c1", + channelId: 1, + user: TEST_USER, + content: "hi", + replyTo: null, + timestamp: "2026-03-15T10:00:00Z", + }); + // Ack arrives first (server sends chat_send_ok before the broadcast). + confirmSend("c1", 555, "2026-03-15T10:00:01Z"); + let msgs = getChannelMessages(1); + expect(msgs).toHaveLength(1); + expect(msgs[0]!.status).toBe("sent"); + expect(msgs[0]!.id).toBe(555); + expect(messagesStore.getState().pendingSends.has("c1")).toBe(false); + + // The broadcast for our own message arrives with the real id → replace, + // not duplicate, upgrading to the full server row. + addMessage(makeChatPayload({ id: 555, content: "hi", attachments: [ATTACHMENT] })); + msgs = getChannelMessages(1); + expect(msgs).toHaveLength(1); + expect(msgs[0]!.id).toBe(555); + expect(msgs[0]!.status).toBe("sent"); + expect(msgs[0]!.correlationId).toBeNull(); + expect(msgs[0]!.attachments).toHaveLength(1); + }); + + it("markSendFailed flips the row to failed with the error code", () => { + addOptimisticMessage({ + correlationId: "c1", + channelId: 1, + user: TEST_USER, + content: "hi", + replyTo: null, + timestamp: "2026-03-15T10:00:00Z", + }); + markSendFailed("c1", "SLOW_MODE"); + const msg = getChannelMessages(1)[0]!; + expect(msg.status).toBe("failed"); + expect(msg.errorCode).toBe("SLOW_MODE"); + expect(messagesStore.getState().pendingSends.has("c1")).toBe(false); + }); + + it("removeOptimistic drops the row (retry / dismiss)", () => { + addOptimisticMessage({ + correlationId: "c1", + channelId: 1, + user: TEST_USER, + content: "hi", + replyTo: null, + timestamp: "2026-03-15T10:00:00Z", + }); + removeOptimistic("c1"); + expect(getChannelMessages(1)).toHaveLength(0); + expect(messagesStore.getState().pendingSends.has("c1")).toBe(false); + }); + + it("addMessage is idempotent by real id (replay-safe)", () => { + addMessage(makeChatPayload({ id: 700, content: "once" })); + addMessage(makeChatPayload({ id: 700, content: "once" })); + expect(getChannelMessages(1)).toHaveLength(1); + }); + + it("defensively reconciles a broadcast that raced ahead of its ack", () => { + addOptimisticMessage({ + correlationId: "c1", + channelId: 1, + user: TEST_USER, + content: "race", + replyTo: null, + timestamp: "2026-03-15T10:00:00Z", + }); + // Broadcast arrives before confirmSend; matched by author against the + // oldest pending row → replaced, not duplicated. + addMessage(makeChatPayload({ id: 800, user: TEST_USER, content: "race" })); + const msgs = getChannelMessages(1); + expect(msgs).toHaveLength(1); + expect(msgs[0]!.id).toBe(800); + expect(msgs[0]!.status).toBe("sent"); + }); + }); }); diff --git a/Client/tauri-client/tests/unit/notifications.test.ts b/Client/tauri-client/tests/unit/notifications.test.ts index 16ab001a..e4084d23 100644 --- a/Client/tauri-client/tests/unit/notifications.test.ts +++ b/Client/tauri-client/tests/unit/notifications.test.ts @@ -124,6 +124,7 @@ describe("notifyIncomingMessage", () => { position: 0, unreadCount: 0, lastMessageId: null, + canSend: true, }, ], ]), diff --git a/Client/tauri-client/tests/unit/renderers.test.ts b/Client/tauri-client/tests/unit/renderers.test.ts index bd00ceaa..7801fd2d 100644 --- a/Client/tauri-client/tests/unit/renderers.test.ts +++ b/Client/tauri-client/tests/unit/renderers.test.ts @@ -40,6 +40,9 @@ function makeMessage(overrides: Partial = {}): Message { editedAt: null, deleted: false, timestamp: "2025-01-15T12:30:00Z", + status: "sent", + correlationId: null, + errorCode: null, ...overrides, }; } @@ -205,6 +208,48 @@ describe("renderers", () => { ac.abort(); }); + it("marks a pending optimistic row and hides the action bar", () => { + const msg = makeMessage({ status: "pending", correlationId: "c1", id: 0 }); + const ac = new AbortController(); + const el = renderMessage(msg, false, [msg], makeOpts(), ac.signal); + container.appendChild(el); + + expect(el.classList.contains("pending")).toBe(true); + // No hover actions on an unsent row. + expect(container.querySelector(".msg-actions-bar")).toBeNull(); + + ac.abort(); + }); + + it("renders a failed row with reason, retry, and discard", () => { + const onRetry = vi.fn(); + const onDeleteDraft = vi.fn(); + const msg = makeMessage({ + status: "failed", + correlationId: "c1", + id: 0, + errorCode: "SLOW_MODE", + }); + const ac = new AbortController(); + const el = renderMessage(msg, false, [msg], makeOpts({ onRetry, onDeleteDraft }), ac.signal); + container.appendChild(el); + + expect(el.classList.contains("failed")).toBe(true); + expect(container.querySelector(".msg-send-failed-text")?.textContent).toContain("Slow mode"); + + const retry = container.querySelector('[data-testid="msg-retry-c1"]'); + const discard = container.querySelector('[data-testid="msg-discard-c1"]'); + expect(retry).not.toBeNull(); + expect(discard).not.toBeNull(); + + retry!.click(); + discard!.click(); + expect(onRetry).toHaveBeenCalledWith("c1"); + expect(onDeleteDraft).toHaveBeenCalledWith("c1"); + + ac.abort(); + }); + it("renders deleted message with italic text", () => { const msg = makeMessage({ deleted: true }); const ac = new AbortController(); diff --git a/Client/tauri-client/tests/unit/screen-share-button.test.ts b/Client/tauri-client/tests/unit/screen-share-button.test.ts index 11e2f300..51548122 100644 --- a/Client/tauri-client/tests/unit/screen-share-button.test.ts +++ b/Client/tauri-client/tests/unit/screen-share-button.test.ts @@ -74,6 +74,7 @@ function setVoiceConnected(screenshare = false): void { position: 0, unreadCount: 0, lastMessageId: null, + canSend: true, }, ], ]), diff --git a/Client/tauri-client/tests/unit/sidebar-area.test.ts b/Client/tauri-client/tests/unit/sidebar-area.test.ts index 39e64799..d6ae3c2d 100644 --- a/Client/tauri-client/tests/unit/sidebar-area.test.ts +++ b/Client/tauri-client/tests/unit/sidebar-area.test.ts @@ -1031,6 +1031,7 @@ describe("SidebarArea", () => { position: 0, unreadCount: 0, lastMessageId: null, + canSend: true, }); return { ...prev, channels: next, activeChannelId: 1 }; }); @@ -1059,6 +1060,7 @@ describe("SidebarArea", () => { position: 0, unreadCount: 0, lastMessageId: null, + canSend: true, }); return { ...prev, channels: next, activeChannelId: 50 }; }); @@ -1268,6 +1270,7 @@ describe("SidebarArea", () => { position: 0, unreadCount: 0, lastMessageId: null, + canSend: true, }); return { ...prev, channels: next, activeChannelId: 1 }; }); @@ -1304,6 +1307,7 @@ describe("SidebarArea", () => { position: 0, unreadCount: 0, lastMessageId: null, + canSend: true, }); next.set(2, { id: 2, @@ -1313,6 +1317,7 @@ describe("SidebarArea", () => { position: 0, unreadCount: 0, lastMessageId: null, + canSend: true, }); return { ...prev, channels: next }; }); @@ -1374,6 +1379,7 @@ describe("SidebarArea", () => { position: 0, unreadCount: 0, lastMessageId: null, + canSend: true, }); return { ...prev, channels: next, activeChannelId: 100 }; }); @@ -1777,6 +1783,7 @@ describe("SidebarArea", () => { position: 0, unreadCount: 0, lastMessageId: null, + canSend: true, }); return { ...prev, channels: next }; }); diff --git a/Client/tauri-client/tests/unit/sidebar-dm-helpers.test.ts b/Client/tauri-client/tests/unit/sidebar-dm-helpers.test.ts index fce393a8..ad469d5a 100644 --- a/Client/tauri-client/tests/unit/sidebar-dm-helpers.test.ts +++ b/Client/tauri-client/tests/unit/sidebar-dm-helpers.test.ts @@ -118,6 +118,7 @@ describe("SidebarDmHelpers", () => { position: 0, unreadCount: 0, lastMessageId: null, + canSend: true, }); return { ...prev, channels: next }; }); @@ -142,6 +143,7 @@ describe("SidebarDmHelpers", () => { position: 0, unreadCount: 0, lastMessageId: null, + canSend: true, }); return { ...prev, channels: next }; }); @@ -172,6 +174,7 @@ describe("SidebarDmHelpers", () => { position: 0, unreadCount: 0, lastMessageId: null, + canSend: true, }); return { ...prev, channels: next, activeChannelId: 1 }; }); @@ -195,6 +198,7 @@ describe("SidebarDmHelpers", () => { position: 0, unreadCount: 0, lastMessageId: null, + canSend: true, }); return { ...prev, channels: next, activeChannelId: 50 }; }); diff --git a/Client/tauri-client/tests/unit/voice-widget.test.ts b/Client/tauri-client/tests/unit/voice-widget.test.ts index 8338789a..32a15be9 100644 --- a/Client/tauri-client/tests/unit/voice-widget.test.ts +++ b/Client/tauri-client/tests/unit/voice-widget.test.ts @@ -118,6 +118,7 @@ describe("VoiceWidget", () => { position: 0, unreadCount: 0, lastMessageId: null, + canSend: true, }); return { ...prev, channels }; }); @@ -150,6 +151,7 @@ describe("VoiceWidget", () => { position: 0, unreadCount: 0, lastMessageId: null, + canSend: true, }); return { ...prev, channels }; }); @@ -182,6 +184,7 @@ describe("VoiceWidget", () => { position: 0, unreadCount: 0, lastMessageId: null, + canSend: true, }); return { ...prev, channels }; }); diff --git a/Server/ws/can_send_ready_test.go b/Server/ws/can_send_ready_test.go new file mode 100644 index 00000000..a527437e --- /dev/null +++ b/Server/ws/can_send_ready_test.go @@ -0,0 +1,46 @@ +package ws_test + +import ( + "encoding/json" + "testing" +) + +// TestBuildReady_IncludesCanSend confirms every ready channel carries the +// can_send affordance flag the client composer keys off. +func TestBuildReady_IncludesCanSend(t *testing.T) { + hub, database := newCoverageHub(t) + user := seedCoverageOwner(t, database, "cansend-user") + role, err := database.GetRoleByID(1) + if err != nil || role == nil { + t.Fatalf("GetRoleByID: %v", err) + } + if _, err := database.CreateChannel("general", "text", "", "", 0); err != nil { + t.Fatalf("CreateChannel: %v", err) + } + msg, err := hub.BuildReadyWithRoleForTest(database, user.ID, role) + if err != nil { + t.Fatalf("BuildReadyWithRoleForTest: %v", err) + } + var env struct { + Payload struct { + Channels []map[string]any `json:"channels"` + } `json:"payload"` + } + if err := json.Unmarshal(msg, &env); err != nil { + t.Fatalf("unmarshal: %v", err) + } + if len(env.Payload.Channels) == 0 { + t.Fatal("expected at least one channel") + } + for _, ch := range env.Payload.Channels { + canSend, ok := ch["can_send"] + if !ok { + t.Errorf("channel %v missing can_send", ch["name"]) + continue + } + // Owner role → can_send true everywhere. + if canSend != true { + t.Errorf("channel %v can_send = %v, want true for owner", ch["name"], canSend) + } + } +} diff --git a/Server/ws/can_send_test.go b/Server/ws/can_send_test.go new file mode 100644 index 00000000..73d65287 --- /dev/null +++ b/Server/ws/can_send_test.go @@ -0,0 +1,75 @@ +package ws + +import ( + "encoding/json" + "testing" + + "github.com/owncord/server/db" + "github.com/owncord/server/permissions" +) + +// TestChannelCanSend locks the composer-gating rule the client relies on: +// it must mirror MessageService.checkSendPermission for non-DM channels. +func TestChannelCanSend(t *testing.T) { + admin := &db.Role{Permissions: permissions.Administrator} + member := &db.Role{Permissions: permissions.ReadMessages | permissions.SendMessages} + reader := &db.Role{Permissions: permissions.ReadMessages} + mod := &db.Role{Permissions: permissions.ReadMessages | permissions.SendMessages | permissions.ManageMessages} + none := db.ChannelOverride{} + + cases := []struct { + name string + role *db.Role + o db.ChannelOverride + ctype string + want bool + }{ + {"nil role fails closed", nil, none, "text", false}, + {"admin bypasses on text", admin, none, "text", true}, + {"admin bypasses on announcement", admin, none, "announcement", true}, + {"member can post in text", member, none, "text", true}, + {"reader without SEND cannot post", reader, none, "text", false}, + {"member without MANAGE cannot post in announcement", member, none, "announcement", false}, + {"moderator can post in announcement", mod, none, "announcement", true}, + {"override deny SEND blocks text", member, db.ChannelOverride{Deny: permissions.SendMessages}, "text", false}, + {"override allow MANAGE enables announcement", member, db.ChannelOverride{Allow: permissions.ManageMessages}, "announcement", true}, + } + for _, c := range cases { + if got := channelCanSend(c.role, c.o, c.ctype); got != c.want { + t.Errorf("%s: channelCanSend = %v, want %v", c.name, got, c.want) + } + } +} + +// TestBuildErrorMsgWithID echoes the request id so the client can correlate a +// failure with the specific command it sent; an empty id omits the field. +func TestBuildErrorMsgWithID(t *testing.T) { + withID := buildErrorMsgWithID(ErrCodeSlowMode, "slow down", "req-42") + var env struct { + Type string `json:"type"` + ID string `json:"id"` + Payload struct { + Code string `json:"code"` + Message string `json:"message"` + } `json:"payload"` + } + if err := json.Unmarshal(withID, &env); err != nil { + t.Fatalf("unmarshal: %v", err) + } + if env.ID != "req-42" { + t.Errorf("id = %q, want req-42", env.ID) + } + if env.Payload.Code != ErrCodeSlowMode { + t.Errorf("code = %q, want %q", env.Payload.Code, ErrCodeSlowMode) + } + + // Empty request id falls back to the id-less envelope. + noID := buildErrorMsgWithID(ErrCodeSlowMode, "slow down", "") + var raw map[string]any + if err := json.Unmarshal(noID, &raw); err != nil { + t.Fatalf("unmarshal noID: %v", err) + } + if _, present := raw["id"]; present { + t.Error("empty reqID should omit the id field") + } +} diff --git a/Server/ws/handlers.go b/Server/ws/handlers.go index 8d21a328..07164a0c 100644 --- a/Server/ws/handlers.go +++ b/Server/ws/handlers.go @@ -103,7 +103,7 @@ func (h *Hub) handleMessage(c *Client, raw []byte) { cmd, parseErr := ctor(c.userID, env.ID, env.Payload) if parseErr != nil { reqLog.Warn("ws command parse error", "err", parseErr) - c.sendMsg(buildErrorMsg(ErrCodeBadRequest, "invalid payload")) + c.sendMsg(buildErrorMsgWithID(ErrCodeBadRequest, "invalid payload", env.ID)) return } @@ -127,15 +127,15 @@ func (h *Hub) handleMessage(c *Client, raw []byte) { result, dispatched := h.registry.DispatchV2(c.ctx, cmd, info) if !dispatched { reqLog.Error("ws V2 handler registered but DispatchV2 returned false", "type", env.Type) - c.sendMsg(buildErrorMsg(ErrCodeInternal, "internal error")) + c.sendMsg(buildErrorMsgWithID(ErrCodeInternal, "internal error", env.ID)) return } if result.Error != nil { if ce, ok := result.Error.(ClientError); ok { - c.sendMsg(buildErrorMsg(ce.Code, ce.Message)) + c.sendMsg(buildErrorMsgWithID(ce.Code, ce.Message, env.ID)) } else { reqLog.Error("ws handler internal error", "err", result.Error) - c.sendMsg(buildErrorMsg(ErrCodeInternal, "internal error")) + c.sendMsg(buildErrorMsgWithID(ErrCodeInternal, "internal error", env.ID)) } return } diff --git a/Server/ws/messages.go b/Server/ws/messages.go index ac316550..8b72a01c 100644 --- a/Server/ws/messages.go +++ b/Server/ws/messages.go @@ -207,6 +207,24 @@ func buildErrorMsg(code, message string) []byte { }) } +// buildErrorMsgWithID produces an error envelope that echoes the originating +// command's request id, so the client can correlate the failure with the +// specific command it sent (e.g. mark an optimistic chat_send row as failed). +// When reqID is empty it falls back to the id-less envelope. +func buildErrorMsgWithID(code, message, reqID string) []byte { + if reqID == "" { + return buildErrorMsg(code, message) + } + return buildJSON(map[string]any{ + "type": MsgTypeError, + "id": reqID, + "payload": map[string]string{ + "code": code, + "message": message, + }, + }) +} + // buildAuthError produces an auth_error envelope per PROTOCOL.md. // The client treats this type as non-recoverable and stops reconnecting. func buildAuthError(message string) []byte { diff --git a/Server/ws/serve.go b/Server/ws/serve.go index 3010d8d0..3740d5c6 100644 --- a/Server/ws/serve.go +++ b/Server/ws/serve.go @@ -564,6 +564,28 @@ func (h *Hub) buildAuthOK(user *db.User, roleName string, replaySource string) [ }) } +// channelCanSend reports whether a user with the given role and per-channel +// override may post in a channel of chanType. It mirrors the non-DM branch of +// MessageService.checkSendPermission so the client can pre-disable the composer +// without a round-trip; the server still enforces the rule authoritatively. +func channelCanSend(role *db.Role, o db.ChannelOverride, chanType string) bool { + if role == nil { + return false + } + if permissions.HasAdmin(role.Permissions) { + return true + } + eff := permissions.EffectivePerms(role.Permissions, o.Allow, o.Deny) + need := permissions.ReadMessages | permissions.SendMessages + if eff&need != need { + return false + } + if chanType == "announcement" { + return eff&permissions.ManageMessages == permissions.ManageMessages + } + return true +} + // buildReady constructs the ready server→client message. // Per PROTOCOL.md, channels include unread_count and last_message_id per user, // and only protocol-specified fields (no slow_mode, archived, voice_* extras). @@ -632,6 +654,12 @@ func (h *Hub) buildReady(database *db.DB, userID int64, role *db.Role) ([]byte, "type": visibleChannels[i].Type, "category": visibleChannels[i].Category, "position": visibleChannels[i].Position, + // can_send drives the client's composer affordance. It mirrors + // MessageService.checkSendPermission for non-DM channels: base role + // ± channel overrides must grant READ|SEND, and announcement + // channels additionally require MANAGE_MESSAGES; admins bypass. The + // server remains the authority — this only pre-disables the UI. + "can_send": channelCanSend(role, overrides[visibleChannels[i].ID], visibleChannels[i].Type), } if visibleChannels[i].Type == "text" || visibleChannels[i].Type == "announcement" { if u, ok := unreadMap[visibleChannels[i].ID]; ok { diff --git a/docs/architecture/ux/README.md b/docs/architecture/ux/README.md index b8b08ca7..0a91fb09 100644 --- a/docs/architecture/ux/README.md +++ b/docs/architecture/ux/README.md @@ -122,11 +122,12 @@ detail each; this is the index. | `server_restart` | `ui.setTransientError` | Restart banner with countdown | | `error` | `ui.setTransientError` (+ `clearAuth` on `BANNED`) | Map the code → the reaction in §5 | -> **⚠ Current gap.** Several codes are received and dropped. `error` handles only -> `BANNED`/`RATE_LIMITED`/`FORBIDDEN`; `SLOW_MODE`, `INVALID_INPUT`, conflict, -> etc. are silently ignored (`dispatcher.ts:421-436`). WS `chat_send_ok` carries -> the real `message_id`/`timestamp` but they are discarded -> (`messages.store.ts:252-258`). Both are addressed in [messaging.md](messaging.md). +> **✓ Implemented (2026-07).** Error codes are no longer silently dropped for +> sends: the server echoes the request id on error replies, so `SLOW_MODE`, +> `FORBIDDEN`, `RATE_LIMITED`, `BAD_REQUEST`, etc. are mapped to the exact +> optimistic row that failed (retry offered), and `chat_send_ok`'s +> `message_id`/`timestamp` now reconcile the pending row. See the optimistic +> lifecycle in [messaging.md](messaging.md). --- diff --git a/docs/architecture/ux/messaging.md b/docs/architecture/ux/messaging.md index 46ac4c6f..dc479031 100644 --- a/docs/architecture/ux/messaging.md +++ b/docs/architecture/ux/messaging.md @@ -61,18 +61,15 @@ stateDiagram-v2 | `slow-mode` | Disabled with a live countdown | "Slow mode: wait Ns." | | `uploading` | Send disabled until uploads settle (already `MessageInput.ts:138-141`) | per-attachment spinner | -> **⚠ Current gap — the composer has no permission/read-only mode.** -> `MessageInput` always renders an enabled textarea (`MessageInput.ts:379-384`); -> the only disabled control is the attach button when uploads aren't wired. There -> is **no** client gating for announcement channels, missing `SEND_MESSAGES`, or -> slow-mode — even though the server enforces all three (announcement requires -> MANAGE_MESSAGES since D1; `ChannelType` `"announcement"` is already threaded to -> `mountChannel`, `ChannelController.ts:114`, but unused). Today the only -> send-time block is "not connected", surfaced as a toast *after* the click -> (`ChannelController.ts:200-204`). Target: derive composer state from -> `permissions` + channel type + connection status and disable with a reason, -> so a forbidden send is never attempted. This needs the client to know the -> user's effective per-channel permission — see the note at the end. +> **✓ Implemented (2026-07).** The server sends an authoritative per-channel +> `can_send` in the ready payload (`ws/serve.go` `channelCanSend`, mirroring +> `MessageService.checkSendPermission`: READ|SEND, plus MANAGE_MESSAGES for +> announcement, admin bypass, channel overrides). `channels.store` carries it as +> `Channel.canSend`; `MessageInput.setDisabled(reason)` disables the composer +> with a visible reason, and `ChannelController` derives that reason from +> `can_send` + channel type + connection status. Older servers that omit +> `can_send` default permissive. Remaining: slow-mode countdown (see §8) and DM +> block-state gating (handled today via the failed-row path in §3). --- @@ -115,17 +112,16 @@ sequenceDiagram `chat_send_ok.id`) is the join key. `addMessage` from the broadcast must detect an existing pending/sent row for that id and replace-in-place rather than append. -> **⚠ Current gap — sending is not optimistic and acks are dropped.** The send -> path fires `chat_send` and does nothing locally; the message appears only when -> the server's `chat_message` broadcast arrives (`ChannelController.ts:199-214`, -> `dispatcher.ts:174-218`). The `pendingSends`/`addPendingSend`/`confirmSend` -> machinery already exists in `messages.store.ts` (`:243-258`) but `addPendingSend` -> has **zero callers**, and `confirmSend` discards the real `message_id`/`timestamp` -> (`messages.store.ts:252`). Transport backpressure ("channel full") is dropped -> silently (`ws.ts:432-437`), and rejection codes other than -> RATE_LIMITED/FORBIDDEN/BANNED are ignored (`dispatcher.ts:433-436`). Target: -> wire the existing pending-send machinery into an optimistic row with -> pending/sent/failed states and a Retry — the store scaffolding is already there. +> **✓ Implemented (2026-07).** `messages.store` now has `addOptimisticMessage` +> (pending row), `confirmSend` (stamps the real id + "sent" on the `chat_send_ok` +> ack), `markSendFailed`, and `removeOptimistic`; `addMessage` reconciles the +> broadcast by real id (idempotent, replay-safe) with a defensive author match. +> `ChannelController.performSend` renders the pending row and `MessageList` +> shows pending (dimmed) and failed (reason + **Retry** / **Delete**) states. +> Failures are precise: the server now echoes the request id on error replies +> (`ws/handlers.go` → `buildErrorMsgWithID`), so the dispatcher's `error` handler +> maps `SLOW_MODE` / `FORBIDDEN` / `RATE_LIMITED` / `BAD_REQUEST` to the exact +> row (`dispatcher.ts`), and an offline send is shown failed rather than dropped. --- @@ -196,24 +192,25 @@ channel's `slow_mode` seconds) and re-enable at zero; on a WS `SLOW_MODE` rejection, snap the composer to the countdown state without dropping the drafted text. -> **⚠ Current gap.** `SLOW_MODE` errors are received but ignored -> (`dispatcher.ts:433-436`); there is no countdown UI. Part of the composer-state -> work in §2. +> **Partially implemented (2026-07).** `SLOW_MODE` errors are now surfaced: they +> mark the optimistic row failed with a "Slow mode — wait before sending again" +> reason and a **Retry** (via the request-id error correlation in §3). The live +> **countdown** in the composer is still outstanding — it needs the channel's +> `slow_mode` seconds, which the ready payload does not yet carry. --- -## Note — the client needs effective per-channel permissions +## Note — effective per-channel permissions (resolved) -Several targets here (§2 composer gating, §4 delete affordance) require the client -to know the user's **effective permission on the active channel** (base role bits -± channel overrides, with the announcement-channel MANAGE_MESSAGES rule). The -client currently receives roles (`ready.roles`) and member roles but does **not** -compute effective per-channel permissions the way the server does -(`Server/permissions`). Delivering the gated composer cleanly likely means either -(a) the server sending a per-channel `can_send`/`permissions` hint (e.g. on -`ready`/`channel_focus`), or (b) porting the permission-bit evaluation to the -client. This is a prerequisite decision for §2 and is flagged as such rather than -hand-waved. +§2's composer gating needs the user's **effective permission on the active +channel** (base role ± channel overrides, with the announcement MANAGE_MESSAGES +rule). This was resolved by **option (a)**: the server sends an authoritative +per-channel `can_send` in the ready payload, computed by `channelCanSend` +(`ws/serve.go`) as a mirror of `MessageService.checkSendPermission`. The client +consumes it directly rather than re-deriving permission math, so overrides and +the announcement rule are always correct. The delete affordance (§4) remains +role-name based; tightening it to effective per-channel permission could reuse +the same signal in future. ---