diff --git a/apps/multiplatform/common/src/desktopMain/kotlin/chat/simplex/common/views/helpers/DefaultDialog.desktop.kt b/apps/multiplatform/common/src/desktopMain/kotlin/chat/simplex/common/views/helpers/DefaultDialog.desktop.kt index 7341c6af23..79aaf5c727 100644 --- a/apps/multiplatform/common/src/desktopMain/kotlin/chat/simplex/common/views/helpers/DefaultDialog.desktop.kt +++ b/apps/multiplatform/common/src/desktopMain/kotlin/chat/simplex/common/views/helpers/DefaultDialog.desktop.kt @@ -14,10 +14,12 @@ import chat.simplex.res.MR import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.launch import java.awt.FileDialog +import java.awt.event.ActionListener import java.io.File import javax.swing.JFileChooser import javax.swing.filechooser.FileFilter import javax.swing.filechooser.FileNameExtensionFilter +import javax.swing.plaf.basic.BasicFileChooserUI @Composable actual fun DefaultDialog( @@ -77,6 +79,13 @@ fun FrameWindowScope.FileDialogChooserMultiple( fileChooser.dialogTitle = title fileChooser.isMultiSelectionEnabled = allowMultiple && isLoad fileChooser.isAcceptAllFileFilterUsed = fileFilter == null + // Only install the glob bypass for the real file-save case (filename != null). When filename + // is null the dialog runs in DIRECTORIES_ONLY mode (e.g. "Save QR code as image"), where the + // Save button must approve the selected directory — the literal-filename handler would instead + // traverse into it (or no-op on an empty field), making directory selection impossible. + if (!isLoad && filename != null && desktopPlatform.isLinux()) { + installUnixSaveGlobBypass(fileChooser) + } if (fileFilter != null && fileFilterDescription != null) { fileChooser.addChoosableFileFilter(object: FileFilter() { override fun accept(file: File?): Boolean = fileFilter(file) @@ -120,6 +129,28 @@ fun FrameWindowScope.FileDialogChooserMultiple( } } +// Replace the Save button's action with a literal-filename handler. This bypasses JFileChooser's +// glob-on-save behaviour, which mis-handles '[' as a glob char on Unix (breaking filenames like +// '[1].pdf') and is not a feature of any native OS save dialog — macOS NSSavePanel and native +// Windows / Linux GTK / KDE save dialogs all treat the typed filename as a literal name. +private fun installUnixSaveGlobBypass(fc: JFileChooser) { + val ui = fc.ui as? BasicFileChooserUI ?: return + val original: ActionListener = ui.approveSelectionAction + val btn = ui.getDefaultButton(fc) ?: return + btn.removeActionListener(original) + btn.addActionListener { + val name = ui.fileName?.takeIf { it.isNotEmpty() } ?: return@addActionListener + val typed = File(name) + val target = if (typed.isAbsolute) typed else File(fc.currentDirectory, name) + if (target.isDirectory && fc.isTraversable(target)) { + fc.currentDirectory = target + } else { + fc.selectedFile = target + fc.approveSelection() + } + } +} + /* * Has graphic glitches on many Linux distributions, so use only on non-Linux systems. Also file filter doesn't work on Windows * */ diff --git a/plans/2026-06-09-support-chat-unread-on-group-read.md b/plans/2026-06-09-support-chat-unread-on-group-read.md new file mode 100644 index 0000000000..5d24cc5ff1 --- /dev/null +++ b/plans/2026-06-09-support-chat-unread-on-group-read.md @@ -0,0 +1,200 @@ +# Fix: support-chat unread stats not cleared after no-scope group read + +Branch: `nd/fix-support-chat-unread-on-group-read` (off `origin/master`) + +## Symptom + +In a group with member admission ("chat with admins"), some members keep showing +as unread in the *chats with members* list even after the admin opens their support +chat and reads everything. Reopening never clears them. It is most visible for +members who have already been given a role (e.g. `member`), because pending members +always render the flag icon anyway, so the stuck badge is invisible for them. + +## Root cause + +The *chats with members* badge is driven by the per-member counters +`support_chat_items_unread / _member_attention / _mentions` on `group_members` +(surfaced as `GroupSupportChat`, `Types.hs:1087`). These are denormalized counters, +maintained by increment on receive and decrement on read. + +Reading a group **without a scope** (`APIChatRead` with `scope = Nothing`, +`Commands.hs:1153-1166`) runs `updateGroupChatItemsRead` (`Store/Messages.hs:2078`), +whose `UPDATE` filtered only by `group_id`: + +```sql +UPDATE chat_items SET item_status = RcvRead, ... +WHERE user_id = ? AND group_id = ? AND item_status = RcvNew +``` + +This flips **support-scope** items to `RcvRead` too, but never touches the +`support_chat_items_*` counters. The result is a permanent desync: + +- the items are read (so the chat "looks" fully read), but +- the per-member counters stay > 0 (so the member stays "unread" in the list), and +- reopening the support chat can't fix it: the scoped read decrement + (`updateGroupScopeUnreadStats`, `Store/Messages.hs:2202`) only fires on a real + `RcvNew -> RcvRead` transition, and there are no `RcvNew` items left. + +This is inconsistent with how the group's main unread **count** is computed, which +already excludes support items (`Store/Messages.hs:916`, +`... AND group_scope_tag IS NULL AND group_scope_group_member_id IS NULL`). The read +path simply forgot the same predicate. + +Note: `member_attention` self-heals on open via `checkSupportChatAttention` +(`Commands.hs:593-613`), recomputing it from loaded items. `unread` and `mentions` +have **no** such correction, which is why the badge persists. + +## Fix (Option A — prevent new corruption) + +Add the main-scope predicate to the two queries in the no-scope group-read path so +that reading a group without a scope leaves support-scope items (and their +disappearing-message timers) entirely untouched: + +- `updateGroupChatItemsRead` (`Store/Messages.hs`) — the read itself. +- `getGroupUnreadTimedItems`, `Nothing` branch (`Store/Messages.hs`) — its companion + timed-items query. + +```sql +AND group_scope_tag IS NULL AND group_scope_group_member_id IS NULL +``` + +Both lines are required and neither is droppable: + +- Without the read filter, support items get marked read (the bug). +- Without the timed-items filter, a disappearing support message would have its + delete timer started by a main-group read while still unread — a *new* + inconsistency introduced by a half-fix. The no-scope `APIChatRead` handler + (`Commands.hs:1160-1166`) calls `getGroupUnreadTimedItems` then + `setGroupChatItemsDeleteAt` then starts deletion threads, so both must agree that + support items are out of scope. + +Support items continue to be read (and their counters decremented) the correct way: +via the scoped read paths (`updateSupportChatItemsRead` / `updateGroupChatItemsReadList` +with a `MemberSupport` scope), exercised when the support chat is opened. + +### Why this is consistent / safe + +- Selects exactly the main-scope rows, identical to the authoritative unread-count + query (`Store/Messages.hs:916`). No main item is ever wrongly excluded — inserts + always set `group_scope_tag` and `group_scope_group_member_id` together + (`Store/Messages.hs:600-603`). +- `groups.members_require_attention` becomes *more* consistent, not less: the + no-scope read no longer zeroes support items behind a still-positive group counter. +- The added predicate is constant, parameterized SQL — no injection surface — and is + the existing codebase idiom (916 / 1214 / 1524 / 1745). + +## Test + +`tests/ChatTests/Groups.hs :: testScopedSupportUnreadStatsGroupReadNoScope` +(registered under the support-chat `describe` block). + +A member sends one support message (`markRead = False`), the admin marks the whole +group read with `/_read chat #1` (no scope), then reads the item in scope with the +**per-item** read `/_read chat items #1(_support:2) `. The per-item read only +decrements on a genuine `RcvNew -> RcvRead` transition, so: + +- on master the no-scope read already consumed the item -> decrement is a no-op -> + `unread: 1, require attention: 1` (test fails); +- with the fix the item is still unread -> decrement applies -> + `unread: 0, require attention: 0` (test passes). + +The full scoped read `/_read chat #1(_support:2)` is deliberately *not* used to +assert this, because `updateSupportChatItemsRead` hard-zeroes the counters +regardless of item state and would mask the bug. + +## Performance + +The added predicate makes both queries **faster**, not slower, and needs no new +index. Verified two ways. + +### Query plan (EXPLAIN QUERY PLAN, via the checked-in plan snapshots) + +Regenerating `chat_query_plans.txt` shows both statements switch index: + +``` +- SEARCH chat_items USING INDEX idx_chat_items_groups_user_mention + (user_id=? AND group_id=? AND item_status=?) -- 3-column seek ++ SEARCH chat_items USING INDEX idx_chat_items_group_scope_stats_all + (user_id=? AND group_id=? AND group_scope_tag=? AND + group_scope_group_member_id=? AND item_status=?) -- 5-column seek +``` + +SQLite compiles `group_scope_tag IS NULL` / `group_scope_group_member_id IS NULL` +into equality constraints (`=?`), so the two new columns *deepen the index seek* +rather than adding a post-seek filter or scan. The read now uses +`idx_chat_items_group_scope_stats_all` +(`user_id, group_id, group_scope_tag, group_scope_group_member_id, item_status, …`, +`M20250721_indexes.hs`), which already exists — this is the same index the +authoritative unread-count query uses. No migration, no new index. + +Note: master was **not** degraded here — it already got a full 3-column seek via +`idx_chat_items_groups_user_mention`. So this is a modest improvement, not the repair +of a regression. The plan file only records two hunks (the two flipped statements); +the `saveQueryPlans` test also churns an unrelated `group_members` count entry and the +whole `agent_query_plans.txt`, both of which were reverted to keep the diff to exactly +those two queries. + +### Micro-benchmark (synthetic 200k-row group) + +On a synthetic group of ~200k `chat_items` where ~25% are support-scope, running the +old vs. new `UPDATE` in alternating trials: + +| Query | mean wall-clock | +|-----------------|-----------------| +| old (no filter) | ~10.5 s | +| new (scoped) | ~7.4 s (≈ −29%) | + +The speedup tracks the row reduction: the new query touches only the ~75% main-scope +rows, so it does ~25% less work and the tighter seek trims a bit more. This also +directly demonstrates the bug — the old query flips ~50k support rows to `RcvRead` +(without decrementing counters); the new one flips **0**. + +Caveat: absolute timings were noisy (concurrent GHC builds on the box); only the +*relative* comparison across alternating trials is reliable, and it was consistent in +direction and magnitude across runs. The gain scales with the support-item share — a +group with no support chats sees no measurable change. + +## Limitations (accepted, by decision) + +1. **No retroactive repair.** This change only prevents *new* corruption. Rows already + stuck (counter > 0 while the items are `RcvRead`) stay stuck until either the + per-member "Mark read" action (`apiSupportChatRead` -> `updateSupportChatItemsRead`, + which hard-zeroes) or, for `member_attention` only, the self-heal on open + (`checkSupportChatAttention`). A one-shot migration recomputing the three counters + from `chat_items` was considered and **intentionally not done**. + +2. **`setUserChatsRead` is not fixed.** The global "mark all chats read" + (`APIUserRead` -> `setUserChatsRead`, `Store/Direct.hs:640-646`) has the same flaw: + its `UPDATE chat_items ... WHERE user_id = ? AND item_status = ?` has no chat-type + and no scope filter, so it reads support items user-wide without decrementing the + counters. It is **left as-is on purpose**: no GUI client invokes `APIUserRead` + (it exists only as the Haskell command type `Controller.hs:357` and the CLI + binding), so it is not reachable from Android / desktop / iOS — only the terminal + CLI. The same one-line predicate would close it if a client ever exposes it. + +3. **`getUpdateGroupItem` relies on a UI contract.** The per-item read + (`updateGroupChatItemsReadList` -> `getUpdateGroupItem`, `Store/Messages.hs:2178`) + has no scope filter on its `UPDATE` and its `Nothing` branch decrements nothing, so + passing a support item ID with `scope = Nothing` would reproduce the desync. This is + defended only by the in-code assumption ("we rely on UI to not pass item IDs from + incorrect scope", `Store/Messages.hs:2162-2163`), not by the query. Not reachable in + normal flow; left unchanged to keep the diff surgical. + +## Files changed + +- `src/Simplex/Chat/Store/Messages.hs` — two one-line scope predicates. +- `tests/ChatTests/Groups.hs` — one regression test + its `it` registration. + +## Build & test run + +Library compiled clean via `bash /home/user/build/linux.sh`. The full test suite was +subsequently built and run against this branch (968 examples). The regression test +`should not read support chat items when reading group without scope` passes with the +fix and fails when the fix is reverted (confirmed by rebuilding master and re-running), +so it genuinely catches the desync. + +The suite had 8 failures on the run; each was classified as flaky/environmental, +by-design, or pre-existing on master — none attributable to this change. In particular +the timed-message failure (`both users have configured timed messages … restart`) was +reproduced identically on master with the fix reverted (3/3), proving it pre-existing; +it is a direct-chat test that never touches the group-scope read path. diff --git a/src/Simplex/Chat/Store/Messages.hs b/src/Simplex/Chat/Store/Messages.hs index 5d6cc8fd8b..7b71e61512 100644 --- a/src/Simplex/Chat/Store/Messages.hs +++ b/src/Simplex/Chat/Store/Messages.hs @@ -2093,6 +2093,7 @@ updateGroupChatItemsRead db User {userId} GroupInfo {groupId} = do [sql| UPDATE chat_items SET item_status = ?, item_viewed = 1, updated_at = ? WHERE user_id = ? AND group_id = ? + AND group_scope_tag IS NULL AND group_scope_group_member_id IS NULL AND item_status = ? |] (CISRcvRead, currentTs, userId, groupId, CISRcvNew) @@ -2149,6 +2150,7 @@ getGroupUnreadTimedItems db User {userId} groupId scope = SELECT chat_item_id, timed_ttl FROM chat_items WHERE user_id = ? AND group_id = ? + AND group_scope_tag IS NULL AND group_scope_group_member_id IS NULL AND item_status = ? AND timed_ttl IS NOT NULL AND timed_delete_at IS NULL |] (userId, groupId, CISRcvNew) diff --git a/src/Simplex/Chat/Store/SQLite/Migrations/chat_query_plans.txt b/src/Simplex/Chat/Store/SQLite/Migrations/chat_query_plans.txt index 36f32944e2..14e2299514 100644 --- a/src/Simplex/Chat/Store/SQLite/Migrations/chat_query_plans.txt +++ b/src/Simplex/Chat/Store/SQLite/Migrations/chat_query_plans.txt @@ -1586,10 +1586,11 @@ Query: SELECT chat_item_id, timed_ttl FROM chat_items WHERE user_id = ? AND group_id = ? + AND group_scope_tag IS NULL AND group_scope_group_member_id IS NULL AND item_status = ? AND timed_ttl IS NOT NULL AND timed_delete_at IS NULL Plan: -SEARCH chat_items USING INDEX idx_chat_items_groups_user_mention (user_id=? AND group_id=? AND item_status=?) +SEARCH chat_items USING INDEX idx_chat_items_group_scope_stats_all (user_id=? AND group_id=? AND group_scope_tag=? AND group_scope_group_member_id=? AND item_status=?) Query: SELECT chat_item_moderation_id, moderator_member_id, created_by_msg_id, moderated_at @@ -5042,10 +5043,11 @@ SEARCH chat_items USING INTEGER PRIMARY KEY (rowid=?) Query: UPDATE chat_items SET item_status = ?, item_viewed = 1, updated_at = ? WHERE user_id = ? AND group_id = ? + AND group_scope_tag IS NULL AND group_scope_group_member_id IS NULL AND item_status = ? Plan: -SEARCH chat_items USING INDEX idx_chat_items_groups_user_mention (user_id=? AND group_id=? AND item_status=?) +SEARCH chat_items USING INDEX idx_chat_items_group_scope_stats_all (user_id=? AND group_id=? AND group_scope_tag=? AND group_scope_group_member_id=? AND item_status=?) Query: UPDATE chat_items SET item_status = ?, item_viewed = 1, updated_at = ? diff --git a/tests/ChatTests/Groups.hs b/tests/ChatTests/Groups.hs index 767728e400..c74b6367b4 100644 --- a/tests/ChatTests/Groups.hs +++ b/tests/ChatTests/Groups.hs @@ -240,6 +240,7 @@ chatGroupTests = do it "should correctly maintain unread stats for support chats on reading chat items" testScopedSupportUnreadStatsOnRead it "should correctly maintain unread stats for support chats on deleting chat items" testScopedSupportUnreadStatsOnDelete it "should correct member attention stat for support chat on opening it" testScopedSupportUnreadStatsCorrectOnOpen + it "should not read support chat items when reading group without scope" testScopedSupportUnreadStatsGroupReadNoScope it "should remove support chat with member when member is removed" testScopedSupportMemberRemoved it "should remove support chat with member when user removes member" testScopedSupportUserRemovesMember it "should remove support chat with member when member leaves" testScopedSupportMemberLeaves @@ -8462,6 +8463,43 @@ testScopedSupportUnreadStatsCorrectOnOpen = { markRead = False } +testScopedSupportUnreadStatsGroupReadNoScope :: HasCallStack => TestParams -> IO () +testScopedSupportUnreadStatsGroupReadNoScope = + testChatOpts2 opts aliceProfile bobProfile $ \alice bob -> do + createGroup2 "team" alice bob + + bob #> "#team (support) 1" + alice <# "#team (support: bob) bob> 1" + -- capture the support item id directly: lastItemId returns the latest item by + -- item_ts, which right after createGroup2 can be a group event ("connected") + -- rather than the support message, making the per-item read below target the + -- wrong item. + bobItemId <- + withCCTransaction alice $ \db -> do + rows <- DB.query_ db "SELECT chat_item_id FROM chat_items WHERE group_scope_tag = 'member_support' ORDER BY chat_item_id DESC LIMIT 1" :: IO [Only Int] + case rows of + Only iId : _ -> pure $ show iId + _ -> error "testScopedSupportUnreadStatsGroupReadNoScope: no member_support item" + + alice ##> "/member support chats #team" + alice <## "members require attention: 1" + alice <## "bob (Bob) (id 2): unread: 1, require attention: 1, mentions: 0" + + -- reading the group without scope must not mark support scope items read + alice #$> ("/_read chat #1", id, "ok") + + -- the support item was left unread, so reading it in scope still decrements the stats + alice #$> ("/_read chat items #1(_support:2) " <> bobItemId, id, "items read for chat") + + alice ##> "/member support chats #team" + alice <## "members require attention: 0" + alice <## "bob (Bob) (id 2): unread: 0, require attention: 0, mentions: 0" + where + opts = + testOpts + { markRead = False + } + testScopedSupportMemberRemoved :: HasCallStack => TestParams -> IO () testScopedSupportMemberRemoved = testChatOpts3 opts aliceProfile bobProfile cathProfile $ \alice bob cath -> do