From 21de23a565c3d3e6fafef2553aad18e2ed8a5151 Mon Sep 17 00:00:00 2001 From: Sivert Date: Wed, 23 Sep 2026 11:32:29 +0200 Subject: [PATCH] Stop holding hidden conversations (GRYT-1379) Hiding a conversation was a row on the server. `dm:setHidden` set `conversation_members.hidden_at`, `listConversationsForUser` filtered those rows out, and a new message cleared it for everybody. Sivert wants it on the device instead. It's a visual thing, and the server shouldn't hold it. So the list stops filtering on `hidden_at`, and `dm:setHidden` and the `dm:hidden` event are gone. A one-shot migration clears every value the old code wrote. Without that clear, anything hidden before today stays filtered out and the new client can never draw it, since it can't show what it isn't sent. The column stays, unused. Drop it and an older binary rolling back writes to a column that isn't there. Keeping it costs one TEXT column per membership row. Blocking used the same column to take the blocked person's conversation off the blocker's list. With the filter gone that write does nothing, so it goes too, and the client hides the row locally instead. Same effect, and you can find it again under Hidden rather than it being gone for good. Hide in a client older than this stops doing anything once a server updates. Co-Authored-By: Claude Opus 5 --- src/db/sqlite/connection.ts | 23 ++++- src/db/sqlite/conversations.ts | 58 +---------- src/socket/handlers/blocks.ts | 10 +- src/socket/handlers/chat.ts | 13 +-- src/socket/handlers/directMessages.test.ts | 111 ++++++++------------- src/socket/handlers/dm.ts | 50 ---------- 6 files changed, 71 insertions(+), 194 deletions(-) diff --git a/src/db/sqlite/connection.ts b/src/db/sqlite/connection.ts index 86d84682..8d8138b5 100644 --- a/src/db/sqlite/connection.ts +++ b/src/db/sqlite/connection.ts @@ -753,8 +753,8 @@ function runMigrations(d: DatabaseSync): void { d.exec("ALTER TABLE conversations ADD COLUMN icon_file_id TEXT"); } - // On the membership row, since it is one person's answer. Nothing is deleted, - // and a new message brings it back. + // Unused since GRYT-1379: hiding a conversation is a client-side, per-device + // thing now. Kept so an older binary rolling back still finds the column. if (!hasColumn(d, "conversation_members", "hidden_at")) { d.exec("ALTER TABLE conversation_members ADD COLUMN hidden_at TEXT"); } @@ -829,6 +829,7 @@ function runMigrations(d: DatabaseSync): void { )`); migrateFileOwnership(d); + clearHiddenConversations(d); d.prepare("UPDATE server_config SET avatar_thumb_px = ?").run(AVATAR_THUMB_PX); @@ -848,6 +849,24 @@ function runMigrations(d: DatabaseSync): void { reissueAccessTokensAfterLeak(d); } +export const HIDDEN_CONVERSATIONS_KEY = "hidden_conversations_cleared"; + +/** + * Anything hidden before GRYT-1379 is still filtered out of the old client's + * list, and the new one cannot show what it is not sent. Once, then stamped. + */ +export function clearHiddenConversations(d: DatabaseSync): void { + if (readSchemaMeta(d, HIDDEN_CONVERSATIONS_KEY)) return; + + const result = d + .prepare(`UPDATE conversation_members SET hidden_at = NULL WHERE hidden_at IS NOT NULL`) + .run(); + if (Number(result.changes) > 0) { + console.log(`[conversations] unhid ${result.changes} conversation membership(s)`); + } + writeSchemaMeta(d, HIDDEN_CONVERSATIONS_KEY, "1"); +} + /** * SQLite cannot widen a primary key in place, so this is the standard rebuild. * A rollback leaves the wider table and an old build's inserts fail loudly. diff --git a/src/db/sqlite/conversations.ts b/src/db/sqlite/conversations.ts index df4f46ec..e70264fb 100644 --- a/src/db/sqlite/conversations.ts +++ b/src/db/sqlite/conversations.ts @@ -77,8 +77,8 @@ export interface ConversationSummary extends ConversationRecord { other_server_user_ids: string[]; } -/* Hidden rows and empty ones are filtered here, so one place decides what a list - holds. `created_at` is the fallback sort, for a conversation opened and unused. */ +/* Empty ones are filtered here, so one place decides what a list holds. + `created_at` is the fallback sort, for a conversation opened and unused. */ export async function listConversationsForUser(serverUserId: string): Promise { const db = getSqliteDb(); const rows = db @@ -87,7 +87,7 @@ export async function listConversationsForUser(serverUserId: string): Promise { - const db = getSqliteDb(); - db.prepare( - `UPDATE conversation_members SET hidden_at = ? - WHERE conversation_id = ? AND server_user_id = ? AND hidden_at IS NULL`, - ).run( - toIso(new Date()), - directConversationId(blockerServerUserId, blockedServerUserId), - blockerServerUserId, - ); -} - -export async function setConversationHidden( - conversationId: string, - serverUserId: string, - hidden: boolean, -): Promise { - const db = getSqliteDb(); - const result = db - .prepare( - `UPDATE conversation_members SET hidden_at = ? - WHERE conversation_id = ? AND server_user_id = ? AND (hidden_at IS NULL) = ?`, - ) - .run(hidden ? toIso(new Date()) : null, conversationId, serverUserId, hidden ? 1 : 0); - return result.changes > 0; -} - -/** Hiding is "not in my sidebar", so without this a hidden conversation swallows - everything sent afterwards. Returns whose list changed. */ -export async function clearConversationHidden(conversationId: string): Promise { - const db = getSqliteDb(); - const rows = db - .prepare( - `SELECT server_user_id FROM conversation_members - WHERE conversation_id = ? AND hidden_at IS NOT NULL`, - ) - .all(conversationId) as { server_user_id: string }[]; - if (rows.length === 0) return []; - - db.prepare(`UPDATE conversation_members SET hidden_at = NULL WHERE conversation_id = ?`).run( - conversationId, - ); - return rows.map((r) => r.server_user_id); -} - export async function touchConversation(conversationId: string, at: Date = new Date()): Promise { const db = getSqliteDb(); db.prepare(`UPDATE conversations SET last_message_at = ? WHERE conversation_id = ?`).run(toIso(at), conversationId); diff --git a/src/socket/handlers/blocks.ts b/src/socket/handlers/blocks.ts index f32ec5fe..a145df5b 100644 --- a/src/socket/handlers/blocks.ts +++ b/src/socket/handlers/blocks.ts @@ -6,7 +6,6 @@ import { unblockUser, listBlocks, getUserByServerId, - hideConversationsBetween, } from "../../db"; import { checkRateLimit, RateLimitRule } from "../../utils/rateLimiter"; @@ -63,13 +62,8 @@ export function registerBlockHandlers(ctx: HandlerContext): EventHandlerMap { await blockUser(auth.tokenPayload.grytUserId, target.gryt_user_id); - /* Only the blocker's list. `hidden_at` is per member and already exists - for closing a conversation, so this is what they could do by hand. */ - await hideConversationsBetween( - auth.tokenPayload.serverUserId, - payload.serverUserId, - ); - + /* The conversation used to be hidden here too. Hiding is the client's + own, per device, since GRYT-1379, so it does that on `user:blocked`. */ socket.emit("user:blocked", { serverUserId: payload.serverUserId }); } catch (err) { consola.error("user:block failed", err); diff --git a/src/socket/handlers/chat.ts b/src/socket/handlers/chat.ts index 28271d05..a7dd963d 100644 --- a/src/socket/handlers/chat.ts +++ b/src/socket/handlers/chat.ts @@ -21,7 +21,6 @@ import { getFilesByIds, getServerConfig, getWebhooksByIds, - clearConversationHidden, getConversation, touchConversation, DEFAULT_UPLOAD_MAX_BYTES, @@ -514,14 +513,10 @@ export function registerChatHandlers(ctx: HandlerContext): EventHandlerMap { consola.warn("touchConversation failed", created.conversation_id, err), ); - // A message unhides a conversation, since hiding only means "not in - // my sidebar". After the send, and swallowed: it cannot block one. + // The first message is what puts the conversation in everybody else's + // list. After the send, and swallowed: it cannot block one. try { - const restored = await clearConversationHidden(created.conversation_id); - const announce = wasEmpty - ? [...new Set([...restored, ...access.memberIds])] - : restored; - for (const serverUserId of announce) { + for (const serverUserId of wasEmpty ? new Set(access.memberIds) : []) { const views = await directConversationViews(serverUserId); const view = views.find((v) => v.conversation_id === created.conversation_id); if (!view) continue; @@ -531,7 +526,7 @@ export function registerChatHandlers(ctx: HandlerContext): EventHandlerMap { } } } catch (err) { - consola.warn("un-hiding the conversation failed", created.conversation_id, err); + consola.warn("announcing the conversation failed", created.conversation_id, err); } } diff --git a/src/socket/handlers/directMessages.test.ts b/src/socket/handlers/directMessages.test.ts index bcc6a0ab..054d1324 100644 --- a/src/socket/handlers/directMessages.test.ts +++ b/src/socket/handlers/directMessages.test.ts @@ -5,7 +5,12 @@ import { join } from "node:path"; import { after, before, describe, it } from "node:test"; import { BOT_SUB_PREFIX } from "../../auth/identity"; -import { initSqlite } from "../../db/sqlite/connection"; +import { + clearHiddenConversations, + getSqliteDb, + HIDDEN_CONVERSATIONS_KEY, + initSqlite, +} from "../../db/sqlite/connection"; import { BUILT_IN_ROLES } from "../../constants/permissions"; import { insertFile } from "../../db/sqlite/messages"; import { createRoleDefinition } from "../../db/sqlite/roleDefinitions"; @@ -561,7 +566,7 @@ describe("a role that may message people but not start groups", () => { }); }); -describe("hiding a conversation", () => { +describe("conversations an older build hid", () => { /** What this member's sidebar would show right now. */ async function listFor(who: Participant): Promise { who.clear(); @@ -570,83 +575,49 @@ describe("hiding a conversation", () => { return list.items.map((i) => i.conversation_id); } - async function hide(who: Participant, conversationId: string, hidden: boolean): Promise { - await who.handlers["dm:setHidden"]({ - accessToken: who.accessToken, - conversationId, - hidden, - }); + /** What a build before GRYT-1379 wrote when somebody hid a conversation. */ + function writeHiddenAt(conversationId: string, serverUserId: string): void { + getSqliteDb() + .prepare(`UPDATE conversation_members SET hidden_at = ? WHERE conversation_id = ? AND server_user_id = ?`) + .run(new Date().toISOString(), conversationId, serverUserId); } - it("takes it out of your list and leaves theirs alone", async () => { - // `hidden_at` is on the membership row, so it is one person's answer. A - // column on `conversations` would hide it for both. - const conversationId = await openDm(alice, bob); - - await hide(alice, conversationId, true); - - assert.equal((await listFor(alice)).includes(conversationId), false, "still in Alice's list"); - assert.equal((await listFor(bob)).includes(conversationId), true, "hiding took it off Bob's too"); - }); - - it("keeps every message", async () => { - const conversationId = await openDm(alice, bob); - await alice.handlers["chat:send"]({ conversationId, accessToken: alice.accessToken, text: "still here" }); - - await hide(alice, conversationId, true); - - clearAll(); - await alice.handlers["chat:fetch"]({ conversationId }); - const history = alice.received("chat:history")[0] as { items: { text: string }[] } | undefined; - assert.ok(history, "hiding took the history away, and it must not"); - assert.ok(history.items.some((m) => m.text === "still here")); - }); - - it("comes back when they say something", async () => { - // Otherwise hiding is a way to never hear from somebody again, and the - // only sign would be an unread count on a row that is not there. - const conversationId = await openDm(alice, bob); - await hide(alice, conversationId, true); - assert.equal((await listFor(alice)).includes(conversationId), false); - - clearAll(); - await bob.handlers["chat:send"]({ conversationId, accessToken: bob.accessToken, text: "you there?" }); - - // Read before `listFor`, which clears what the socket has seen. - const toldAlice = alice.received("dm:opened").some( - (v) => (v as { conversation_id?: string }).conversation_id === conversationId, - ); - - assert.equal((await listFor(alice)).includes(conversationId), true, "stayed hidden through a new message"); - assert.ok(toldAlice, "Alice was never told it came back, so her sidebar would not know"); - }); - - it("comes back when you open it again", async () => { - const conversationId = await openDm(alice, bob); - await hide(alice, conversationId, true); - - await alice.handlers["dm:open"]({ accessToken: alice.accessToken, targetServerUserId: bob.serverUserId }); - - assert.equal((await listFor(alice)).includes(conversationId), true); + it("no longer takes dm:setHidden", async () => { + // Hiding is the client's own now, per device. An older client emitting this + // gets no answer, which is the accepted cost of the server letting go. + assert.equal(alice.handlers["dm:setHidden"], undefined); }); - it("can be put back by hand", async () => { + it("lists one that still carries hidden_at", async () => { + // The list stopped filtering on the column, so a row an older build hid is + // sent again and the client decides whether to draw it. const conversationId = await openDm(alice, bob); - await hide(alice, conversationId, true); - await hide(alice, conversationId, false); + writeHiddenAt(conversationId, alice.serverUserId); - assert.equal((await listFor(alice)).includes(conversationId), true); + assert.equal((await listFor(alice)).includes(conversationId), true, "still filtered out of the list"); }); - it("refuses a conversation that is not yours", async () => { + it("clears the column once, and leaves the marker behind", async () => { const conversationId = await openDm(alice, bob); - clearAll(); - - await hide(mallory, conversationId, true); - - assert.ok(mallory.received("dm:error").length > 0, "no refusal"); - assert.equal((await listFor(alice)).includes(conversationId), true, "Mallory hid somebody else's conversation"); - assert.equal((await listFor(bob)).includes(conversationId), true); + writeHiddenAt(conversationId, alice.serverUserId); + + const db = getSqliteDb(); + db.prepare(`DELETE FROM schema_meta WHERE key = ?`).run(HIDDEN_CONVERSATIONS_KEY); + clearHiddenConversations(db); + + const row = db + .prepare(`SELECT hidden_at FROM conversation_members WHERE conversation_id = ? AND server_user_id = ?`) + .get(conversationId, alice.serverUserId) as { hidden_at: string | null } | undefined; + assert.equal(row?.hidden_at, null, "the migration left a hidden_at behind"); + + // Stamped, so it never runs again — and a rollback to an older build finds + // the column still there rather than a missing one. + writeHiddenAt(conversationId, alice.serverUserId); + clearHiddenConversations(db); + const again = db + .prepare(`SELECT hidden_at FROM conversation_members WHERE conversation_id = ? AND server_user_id = ?`) + .get(conversationId, alice.serverUserId) as { hidden_at: string | null } | undefined; + assert.notEqual(again?.hidden_at, null, "the one-shot ran a second time"); }); }); diff --git a/src/socket/handlers/dm.ts b/src/socket/handlers/dm.ts index 1d5b15ee..f5a94051 100644 --- a/src/socket/handlers/dm.ts +++ b/src/socket/handlers/dm.ts @@ -15,7 +15,6 @@ import { MAX_CONVERSATION_MEMBERS, purgeOrphanedConversations, openDirectConversation, - setConversationHidden, setConversationIcon, setConversationName, eitherHasBlocked, @@ -199,10 +198,6 @@ export function registerDirectMessageHandlers(ctx: HandlerContext): EventHandler const conversation = await openDirectConversation(self, target); - // Asking for a hidden conversation is asking for it back. The caller's - // row only; a message is what brings the other party's back. - await setConversationHidden(conversation.conversation_id, self, false); - /* The other end only once there is something to tell them about. An empty conversation is in nobody's list but its opener's. */ const audience = conversation.last_message_at ? [self, target] : [self]; @@ -222,51 +217,6 @@ export function registerDirectMessageHandlers(ctx: HandlerContext): EventHandler } }, - /** `hidden_at` is on the caller's own row and a new message brings it back, - so this is not a way to stop somebody talking to you. */ - 'dm:setHidden': async (payload: { accessToken: string; conversationId: string; hidden: boolean }) => { - try { - const ip = getClientIp(); - const rl = checkRateLimit("dm:setHidden", clientsInfo[clientId]?.serverUserId, ip, RL_OPEN); - if (!rl.allowed) { - socket.emit("dm:error", { error: "rate_limited", retryAfterMs: rl.retryAfterMs, message: `Too fast. Wait ${Math.ceil((rl.retryAfterMs || 0) / 1000)}s.` }); - return; - } - - if (!payload || typeof payload.accessToken !== "string" || typeof payload.conversationId !== "string" || typeof payload.hidden !== "boolean") { - socket.emit("dm:error", { error: "invalid_payload", message: "Invalid payload" }); - return; - } - - const auth = await requireAuth(socket, payload); - if (!auth) return; - - const self = auth.tokenPayload.serverUserId; - - // The membership check is what stops a row being written for somebody - // else's conversation if the table ever gains a different key. - const conversation = await getConversation(payload.conversationId); - if (!conversation || !(await isConversationMember(payload.conversationId, self))) { - socket.emit("dm:error", { error: "not_found", message: "No such conversation" }); - return; - } - - await setConversationHidden(payload.conversationId, self, payload.hidden); - - // Every socket this person has, so hiding on the desktop takes it off - // the phone as well. - for (const cid of socketIdsFor(self)) { - io.sockets.sockets.get(cid)?.emit("dm:hidden", { - conversation_id: payload.conversationId, - hidden: payload.hidden, - }); - } - } catch (err) { - consola.error("dm:setHidden failed", err); - socket.emit("dm:error", { error: "failed", message: "Could not update the conversation" }); - } - }, - /** Adding somebody to a one-to-one does not happen: this makes a new id and the pair conversation stays as it was. */ 'dm:group:create': async (payload: { accessToken: string; memberIds: string[]; name?: string; iconFileId?: string }) => {