From c3d26263ddccce97cc137c3d479f148170db5f59 Mon Sep 17 00:00:00 2001 From: luongtrieuvy202 Date: Tue, 22 Sep 2026 00:18:28 +0700 Subject: [PATCH 01/34] fix(contact): seed the invite handshake into the DM, not the workspace channel MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit contact.invite_accept greets a brand-new contact from both sides (_contact_invite_chat_msg / _contact_accept_chat_msg). handshake() posted both through channel_post_message into the two drumates' legacy `channel` tables, which breaks the invariant patches/changelog.txt states: `channel` is one table per WORKSPACE, `p2p_channel` one per USER. Two consequences. The greeting never reached the conversation it was for — the contact chat reads p2p_channel (chat.messages -> p2p_list_messages) and was never looking at `channel`. And it leaked into workspace chat: the drumate branch of channel_post_message does not persist entity_id at all (the peer survives only in time_channel), so channel_list_messages cannot filter by peer, and since the personal hub's id IS the viewer's uid, its window's "Team Chat" runs channel.messages straight against the drumate DB. Every contact's handshake, from every contact, rendered as one conversation. Post it the way chat.post does instead (chat.js _distributeMessage): a single p2p_post_message write in the author's own DB with peer_id set, the SP updating the recipient's p2p_time cross-DB. Both SPs, and count_yet_read_next, are already deployed to every drumate DB. The chat.post WS push is oriented per recipient — peer_id is the sender from the recipient's side, which is what their widget matches its peerId against — and carries the author resolved through shareroom_contact_get, so a bubble landing on an already-open conversation renders with a name. No echoId: a server-written greeting echoes no pending bubble. The legacy acknowledge_message call is gone; p2p_post_message marks the author's own copy seen. Co-Authored-By: Claude Opus 5 (1M context) --- service/private/contact.js | 105 ++++++++++++++++++++++++++----------- 1 file changed, 73 insertions(+), 32 deletions(-) diff --git a/service/private/contact.js b/service/private/contact.js index ca82b90..2215d86 100644 --- a/service/private/contact.js +++ b/service/private/contact.js @@ -1534,41 +1534,82 @@ class __private_contact extends Contact { /** - * - * @param {*} message - * @param {*} uid - * @param {*} entity_id + * Seeds a brand-new contact conversation with one canned greeting. + * + * `message` is authored by `uid` and addressed to `entity_id`. It is a SINGLE + * write into the author's OWN drumate DB (`p2p_channel`, via + * p2p_post_message) — the model chat.post uses, see chat.js + * `_distributeMessage` — because `p2p_channel` is the table the 1:1 contact + * chat reads (`p2p_list_messages`, which unions both peers' DBs). + * + * It used to post through `channel_post_message` into BOTH drumates' legacy + * `channel` tables. That table has no peer column — in the drumate branch the + * SP does not persist `entity_id` at all — so the greeting never surfaced in + * the contact conversation, and instead piled up in the personal hub's + * channel list, where the workspace "Team Chat" panel (channel.messages -> + * channel_list_messages, which cannot filter by peer) rendered every + * contact's handshake, from every contact, as one conversation. + * + * @param {*} message canned greeting, already resolved for the language + * @param {*} uid author of the greeting + * @param {*} entity_id the peer it is addressed to */ async handshake(message, uid, entity_id) { - let input = {}; - let myinput = {}; - - let hisinput = {}; - let mydata = {}; - let hisdata = {}; - let acknowledge = {}; let message_id = await this.db.await_proc('message_id'); - message_id = message_id.id - input.author_id = uid - input.uid = uid - input.message_id = message_id - hisinput = input - myinput = input - - myinput.entity_id = entity_id - mydata = await this.yp.await_proc('forward_proc', uid, 'channel_post_message', `'${stringify(myinput)}','${message}'`) - hisinput.entity_id = uid - hisdata = await this.yp.await_proc('forward_proc', entity_id, 'channel_post_message', `'${stringify(hisinput)}','${message}'`) - - acknowledge.message_id = message_id - acknowledge.entity_id = entity_id - acknowledge.uid = uid - await this.yp.await_proc('forward_proc', uid, 'acknowledge_message', `'${stringify(acknowledge)}'`) - - mydata.to_id = uid; - mydata.echoId = this.input.get('echoId'); - hisdata.to_id = entity_id - let service = "chat.post"; + message_id = message_id.id; + if (!isEmpty(message)) { + message = message.replace(/'/gi, "''"); + } + const input = { + author_id: uid, + uid, + peer_id: entity_id, + message_id, + }; + const data = await this.yp.await_proc( + 'forward_proc', uid, 'p2p_post_message', + `'${stringify(input)}','${message}'` + ); + // A failing SP answers {SUCCESS:0, ERROR:{...}} rather than a row, so the + // absence of message_id — not emptiness — is the failure signal. + if (isEmpty(data) || isEmpty(data.message_id)) { + this.warn('[CONTACT] handshake post failed', uid, entity_id, stringify(data)); + return; + } + data.is_attachment = 0; + // No echoId: this greeting is written by the server, so it echoes nothing + // either session posted. Forwarding the invite_accept request's id would + // only risk _.uniqueId() colliding with a bubble a session has pending. + + const service = "chat.post"; + // Author's own sessions: the peer of this conversation is the recipient. + const mycount = await this.yp.await_proc( + 'forward_proc', uid, 'count_yet_read_next', `'${uid}','${entity_id}'` + ) || {}; + const mydata = { + ...data, to_id: uid, peer_id: entity_id, + room: mycount.room, total: mycount.total, + }; + // Recipient's sessions: from their side the peer is the author, which is + // what their chat widget matches its own peerId against + // (chat/index.js -> `privateMach`). It also needs the author resolved, as + // chat.messages and chat.post both hand it one — without it a bubble that + // lands while the conversation is already open renders with no name. + const hiscount = await this.yp.await_proc( + 'forward_proc', entity_id, 'count_yet_read_next', `'${entity_id}','${uid}'` + ) || {}; + const hisdata = { + ...data, to_id: entity_id, peer_id: uid, + room: hiscount.room, total: hiscount.total, + }; + try { + hisdata.entity = await this.yp.await_proc( + 'forward_proc', entity_id, 'shareroom_contact_get', `'${uid}'` + ); + } catch (error) { + this.warn('[CONTACT] handshake author lookup failed:', error.message); + } + let sockets = await this.yp.await_proc('user_sockets', entity_id); await RedisStore.sendData(this.payload(hisdata, { service }), sockets); sockets = await this.yp.await_proc('user_sockets', uid); From 7ffc5c8dd2e4fb17046ca554657c79d316a51fe1 Mon Sep 17 00:00:00 2001 From: luongtrieuvy202 Date: Tue, 22 Sep 2026 00:29:20 +0700 Subject: [PATCH 02/34] fix(task): reclaim an uploaded attachment once its last link goes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Task attachments now upload into the hub's hidden task folder (/__chat__/__task__) instead of the workspace body, so that they stop appearing in the folder's Files tab. That closes one hole and opens another: the file used to stay visible in the folder after an unlink, where the user could still delete it, and would now sit in a folder no listing shows with no way for anyone to reclaim the space. _purgeUnlinkedFiles drops the media node once neither task_file nor task_comment_file names it any more, from all four paths that can remove the last link: unlink_file, comment_unlink_file, delete and comment_delete. The last two read the nids BEFORE the SP removes the rows that name them. Only files UNDER /__chat__/__task__ are touched, so an attachment linked from the workspace body — a document in its own right — is left exactly where it is. mfs_attachment_remove re-checks '^/__chat__' itself, so a wrong nid reaching here still cannot delete someone's file, and comment_delete purges only when affected says something was actually removed, or a non-author could take another's attachment with it. It never throws: failing to reclaim a file must not fail the unlink or the delete the user actually asked for. Attachments already sitting in folder bodies match none of this and are untouched. Co-Authored-By: Claude Opus 5 (1M context) --- service/private/task.js | 127 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 124 insertions(+), 3 deletions(-) diff --git a/service/private/task.js b/service/private/task.js index 4dc3536..845c30c 100644 --- a/service/private/task.js +++ b/service/private/task.js @@ -17,7 +17,8 @@ const { Attr, RedisStore, toArray } = require('@drumee/server-essentials'); const { isEmpty } = require('lodash'); -const { Entity } = require('@drumee/server-core'); +const { Entity, MfsTools } = require('@drumee/server-core'); +const { remove_node } = MfsTools; const { notifyTaskEvent } = require('../lib/activity-mailer'); const {admit: admitMobilePush} = require('../lib/mobile-push'); const { markFeatureUsage } = require('../lib/feature-usage'); @@ -936,7 +937,10 @@ class __private_task extends Entity { const meta = await this._taskColMeta(id); // Log BEFORE the delete — task_activity_log snapshots the task's nid/title. await this._logActivity(id, 'update', { deleted: 1 }); + // Read the attachments BEFORE task_delete drops the rows that name them. + const attached = await this._taskAttachedNids(id); const data = await this.db.await_proc('task_delete', id); + await this._purgeUnlinkedFiles(attached); const row = Array.isArray(data) ? data[0] : data; // The SP returns the children as a comma-separated string (GROUP_CONCAT), // NULL when there were none. Normalise to an array so the client never has @@ -949,6 +953,112 @@ class __private_task extends Entity { this.output.data(result); } + /** + * Drop an attachment's media node once nothing points at it any more. + * + * Only files the task panel UPLOADED are touched. Those live in the hub's + * hidden task folder (/__chat__/__task__ — see mfs_home), which exists so an + * attachment does not appear in the workspace's Files tab beside the real + * documents. A file LINKED from the workspace body is a document in its own + * right and is left exactly where it is; the file_path test below is what + * tells the two apart, and mfs_attachment_remove re-checks '^/__chat__' + * itself, so a wrong nid reaching here still cannot delete someone's file. + * + * Without this, an attachment outlived every task that referenced it, in a + * folder no listing shows, with no way for anyone to reclaim the space. + * + * NEVER THROWS: failing to reclaim a file must not fail the unlink or the + * delete that the user actually asked for. + */ + async _purgeUnlinkedFiles(file_nids) { + const nids = [...new Set(toArray(file_nids).map(String).filter(Boolean))]; + if (!nids.length) return; + let home; + try { + home = await this.db.call_proc('mfs_home'); + } catch (err) { + this.warn('task: mfs_home failed, keeping orphan attachments', err && err.message); + return; + } + if (!home || !home.home_dir) return; + for (const nid of nids) { + try { + // Still referenced by another task, or by a comment on one? Then it is + // not an orphan. Both tables are checked: a file can be attached to the + // task AND quoted in a comment on it, and the last reference wins. + const refs = await this.db.await_query( + 'SELECT 1 AS n FROM task_file WHERE file_nid=? LIMIT 1', + `${nid}` + ); + if (!isEmpty(toArray(refs))) continue; + const crefs = await this.db.await_query( + 'SELECT 1 AS n FROM task_comment_file WHERE file_nid=? LIMIT 1', + `${nid}` + ); + if (!isEmpty(toArray(crefs))) continue; + const rows = await this.db.await_query( + 'SELECT file_path FROM media WHERE id=?', + `${nid}` + ); + const node = toArray(rows)[0]; + const path = (node && node.file_path) || ''; + if (!/^\/__chat__\/__task__\//.test(`${path}`)) continue; + await this.db.await_proc('mfs_attachment_remove', `${nid}`); + await remove_node({ + nid, + hub_id: this.hub && this.hub.get(Attr.id), + mfs_root: `${home.home_dir}/__storage__/`, + }); + } catch (err) { + this.warn('task: failed to purge orphan attachment', nid, err && err.message); + } + } + } + + /** + * Every media nid a task and its subtasks point at, from both the task's own + * attachments and its comments' — read BEFORE task_delete removes the rows + * that name them, so _purgeUnlinkedFiles still has something to check. + */ + async _taskAttachedNids(task_id) { + try { + const rows = await this.db.await_query( + `SELECT file_nid FROM task_file + WHERE task_id = ? + OR task_id IN (SELECT id FROM task WHERE parent_task_id = ?) + UNION + SELECT cf.file_nid FROM task_comment_file cf + JOIN task_comment c ON c.id = cf.comment_id + WHERE c.task_id = ? + OR c.task_id IN (SELECT id FROM task WHERE parent_task_id = ?)`, + `${task_id}`, `${task_id}`, `${task_id}`, `${task_id}` + ); + return toArray(rows).map((r) => r && r.file_nid).filter(Boolean); + } catch (err) { + this.warn('task: could not read attachments before delete', err && err.message); + return []; + } + } + + /** + * Every media nid a comment thread points at — the root and its replies, the + * same set task_comment_delete removes the links for. + */ + async _commentAttachedNids(comment_id) { + try { + const rows = await this.db.await_query( + `SELECT cf.file_nid FROM task_comment_file cf + JOIN task_comment c ON c.id = cf.comment_id + WHERE c.id = ? OR c.parent_id = ?`, + `${comment_id}`, `${comment_id}` + ); + return toArray(rows).map((r) => r && r.file_nid).filter(Boolean); + } catch (err) { + this.warn('task: could not read comment attachments before delete', err && err.message); + return []; + } + } + /** * Link a file (media nid) to a task. Idempotent (INSERT IGNORE). * Params: task_id (required), file_nid (required) @@ -977,6 +1087,8 @@ class __private_task extends Entity { const file_nid = this.input.need('file_nid'); const data = await this.db.await_proc('task_unlink_file', task_id, file_nid); + // Reclaim the node if that was its last reference — see _purgeUnlinkedFiles. + await this._purgeUnlinkedFiles([file_nid]); const result = { task_id, file_nid, ...data }; await this._broadcast('task.unlink_file', result); this.output.data(result); @@ -1177,8 +1289,14 @@ class __private_task extends Entity { async comment_delete() { const id = this.input.need('id'); const task_id = this.input.need('task_id'); + // The SP takes the whole thread (root + replies) and their file links with + // it, so collect the nids while the links still name them. + const attached = await this._commentAttachedNids(id); const data = await this.db.await_proc('task_comment_delete', id, this.uid); const row = Array.isArray(data) ? data[0] : data; + // affected = 0 means a non-author asked: nothing was deleted, nothing to + // reclaim — and purging here would let anyone delete another's attachment. + if (row && row.affected) await this._purgeUnlinkedFiles(attached); const result = { id, task_id, @@ -1266,8 +1384,10 @@ class __private_task extends Entity { } /** - * Detach a file from one's own comment. The media node itself is untouched — - * it lives in the folder body, exactly as with unlink_file. + * Detach a file from one's own comment. A file LINKED from the workspace body + * is untouched and stays where it is; one the panel uploaded into the hidden + * task folder is reclaimed once nothing else points at it — exactly as with + * unlink_file (see _purgeUnlinkedFiles). * Params: comment_id, file_nid, task_id (required, for the broadcast). */ async comment_unlink_file() { @@ -1285,6 +1405,7 @@ class __private_task extends Entity { this.uid ); const row = Array.isArray(data) ? data[0] : data; + await this._purgeUnlinkedFiles([file_nid]); const result = { comment_id, file_nid, task_id, affected: row && row.affected }; await this._broadcast('task.comment_update', { ...comment, task_id }); this.output.data(result); From 4a62a2621f279886d04f234ec1cef146e6b9b875 Mon Sep 17 00:00:00 2001 From: EddyOne81 Date: Tue, 22 Sep 2026 06:34:07 -0700 Subject: [PATCH 03/34] feat(invite): an invitation you answer, instead of a membership you find MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Inviting somebody who already had a Drumee account added them to the workspace on the spot. They were never asked: the first they knew of it was a workspace appearing in their sidebar, and the email that followed announced something already done. There was no accept and no decline because there was nothing left to answer. Both branches of hub.invite now mint an invitation and stop. Membership is written by accept_invite, when the person says yes -- from the email's Accept link or from the notification row. WHAT IS LEFT OF THE ACCOUNT-STATUS BRANCH is one thing: whether there is a Drumee user to notify. The token, the pending row, the audit line, the tracking and the email are now identical for both, which is the point -- two half-shaped invitations that had to be told apart everywhere downstream are one. hub.decline_invite is new and takes NO SESSION. Holding the secret is the authorisation, exactly as it already was for redemption, and this is the strictly less dangerous of the two: accepting with a secret grants access to a workspace, declining only destroys an invitation addressed to one address. Requiring a sign-in would mean the one answer that needs no account could only be given by creating one. It never removes membership -- that is desk.leave_hub -- and the proc refuses anything that is not an ACTIVE hub_invite token, so a stale link pressed after joining cannot rewrite how somebody got in. hub.invitations backs the Access panel's Pending Invitations section. Current members are filtered out HERE rather than in the proc, because membership lives in the workspace database while hub_invitations reads yellow_page -- the case it catches is an address invited while an older invitation was still open, then added directly by add_contributors, which asks nobody. 🚨 THE CHAT STAGING GRANT, which accept_invite has never written. A member whose role is Chat has no write bit for the workspace and is meant to get write on the hidden '/__chat__/__upload__' folder alone. _grantMembership writes that grant; redeeming a token did not, so anybody who joined by link could not attach a file to a chat -- the same 403 that was chased and fixed on the other path (schemas 2026-09-17, chat_upload_grant_repair). Survivable while redemption was the rare branch; it is now how every member joins, so leaving it out would reintroduce that bug as the normal case on the one path nobody had looked at. _trackInviteSent now picks its procedure and the CALLER must say which. invite() passes answerable:true for invite_track_mark_v2, which leaves accept_time NULL. The default stays the old procedure because invite_with_roles still grants immediately -- a default that changed under it would record every one of its grants as an invitation nobody answered, with the same arity, no error and a wrong number. _closeInvitation is the cleanup both answers share. The one that matters is pending_invitation: that table is the QUEUE signup grants membership from, not a record, so a refusal that only marked the token would be undone by the invitee signing up afterwards -- declining a workspace and then finding themselves in it. The notification row carries the token out as `invite_token` on all THREE surfaces the same row reaches: hub.invite_received_get, the Unread-ON rollup (mapHubInviteRow) and the default feed's raw row (_stampHubInvites). Missing one is how file notifications got a dead click in September. Rows written by _grantMembership carry no token and correctly render without buttons -- they are a receipt, not an invitation. The email offers both answers and stops claiming the deed is done: subject, title, headline, body and footer all said "added you to" or "now have access", which were true only while this mail followed a membership that had already been written. Verified by rendering the template both ways -- internal and external -- with no stale phrase left outside an HTML comment, one accept link, one decline link, and the & correctly escaped. 🚨 lodash templates have no comment delimiter. An EJS-style comment tag compiles as evaluate and throws SyntaxError, taking every invitation email with it -- hit while writing the comment that now warns about it, and the warning could not spell the tag out either, because lodash scans the whole file including HTML comments. Co-Authored-By: Claude Opus 5 (1M context) --- acl/hub.json | 51 +- service/private/activity.js | 16 + service/private/hub.js | 619 ++++++++++++++---- .../butler/workspace-invite-member.html | 68 +- 4 files changed, 624 insertions(+), 130 deletions(-) diff --git a/acl/hub.json b/acl/hub.json index d8ef1fa..7dd1f3e 100644 --- a/acl/hub.json +++ b/acl/hub.json @@ -176,8 +176,57 @@ } }, + "decline_invite": { + "doc": "Turn down a workspace invitation, by the token from its email link or its notification row. Holding the secret is the authorisation, exactly as it is for accept_invite - and this is the less dangerous of the two, since accepting grants access while declining only destroys an invitation. No session is required: an invitation usually goes to somebody with no account, and saying no must not cost them a signup. Never removes membership (that is desk.leave_hub) and refuses any token that is not an ACTIVE hub_invite, so a stale link pressed after joining cannot rewrite how somebody got in. Idempotent - declining twice keeps the first answer and reports the same result.", + "scope": "hub", + "permission": { + "src": "anonymous", + "fast_check": "public-api" + }, + "params": { + "token": { + "type": "string", + "required": true, + "doc": "The invite token secret from the email link or the notification row" + } + }, + "returns": { + "status": { "type": "string", "doc": "declined on success; invalid when the token is not a live hub invite; already_used when it was already accepted" }, + "hub_id": { "type": "string", "doc": "Workspace the invitation was for, when the token resolved one" } + } + }, + + "invitations": { + "doc": "Invitations this workspace is still waiting on, plus the ones that were turned down - the Pending Invitations section of the Access panel. One row per person, newest invitation wins. Accepted invitations are absent (those people are members) and so are lapsed ones. Current members are filtered out here rather than in the proc, because membership lives in the workspace database and hub_invitations reads yellow_page.", + "scope": "hub", + "permission": { + "src": "admin" + }, + "params": {}, + "returns": { + "type": "array", + "doc": "Pending and declined invitations", + "items": { + "email": { "type": "string", "doc": "Address the invitation was sent to" }, + "status": { "type": "string", "doc": "pending | declined" }, + "ctime": { "type": "integer", "doc": "Unix timestamp the invitation was sent" }, + "permission": { "type": "number", "doc": "Privilege bitmask the invitation offers" }, + "invitee_uid": { "type": "string", "doc": "Invitee's Drumee user id, null when they have no account yet" }, + "invitee_firstname": { "type": "string", "doc": "Invitee's first name when they have an account" }, + "invitee_lastname": { "type": "string", "doc": "Invitee's last name when they have an account" }, + "invitee_fullname": { "type": "string", "doc": "Invitee's display name when they have an account" }, + "invitee_avatar": { "type": "string", "doc": "Invitee's avatar when they have an account" }, + "inviter_id": { "type": "string", "doc": "Who sent it" }, + "inviter_firstname": { "type": "string", "doc": "Inviter's first name" }, + "inviter_lastname": { "type": "string", "doc": "Inviter's last name" }, + "inviter_fullname": { "type": "string", "doc": "Inviter's display name" } + } + }, + "errors": [] + }, + "invite": { - "doc": "Invite one or more emails into this workspace, branching on area (share-link vs restricted) and drumate status.", + "doc": "Invite one or more emails into this workspace. Mints an invitation the recipient answers with Accept or Decline - it does NOT add anybody. An address that already has an account additionally gets a notification row carrying the token; every address gets the email. Membership is written by accept_invite.", "scope": "hub", "permission": { "src": "admin" diff --git a/service/private/activity.js b/service/private/activity.js index f82970b..2897d9c 100644 --- a/service/private/activity.js +++ b/service/private/activity.js @@ -286,6 +286,14 @@ function mapHubInviteRow(r) { // Shared with hub.invite_received_get so the two surfaces cannot drift // apart again — that drift is what left this one rendering a blank name. hub_name: resolveHubInviteName(r, meta), + // The invitation's own secret, which is what lets the row offer Accept and + // Decline. Written only by hub.invite (_notifyInvitee); the row + // _grantMembership writes when an admin adds somebody directly is a receipt + // for a membership that already exists, carries no token, and correctly + // renders without buttons. Surfaced under the same name as in + // hub.invite_received_get so the bell and the invitations list cannot + // disagree about whether a row can be answered. + invite_token: meta.token || null, }; } @@ -1541,6 +1549,14 @@ class MfsActivity extends Entity { if (!r.category) r.category = 'hub_invite'; if (r.hub_id == null && meta.hub_id != null) r.hub_id = meta.hub_id; if (r.author_id == null && r.uid != null) r.author_id = r.uid; + // THE THIRD SURFACE THE SAME ROW REACHES, and it has to agree with the + // other two. mapHubInviteRow (the Unread-ON rollup) and + // hub.invite_received_get both carry the token out as `invite_token`; + // this is the raw contact_activity row the DEFAULT feed serves, and an + // invitation that arrived through it would otherwise render without its + // Accept and Decline buttons — the same one-event-two-paths split that + // left file notifications with a dead click in September. + if (r.invite_token == null && meta.token != null) r.invite_token = meta.token; targets.push([r, meta]); if (r.hub_name == null && meta.hub_id && wanted.size < MAX_LOOKUPS) { wanted.add(meta.hub_id); diff --git a/service/private/hub.js b/service/private/hub.js index 24563ef..fd6d8ce 100644 --- a/service/private/hub.js +++ b/service/private/hub.js @@ -365,6 +365,48 @@ class __private_hub extends Hub { return `${base}?${q.join("&")}`; } + /** + * The Accept and Decline links in an invitation email. + * + * Built on the SAME route the CTA uses, and carrying the same `?invite=` + * parameter the front end has understood since the token flow shipped + * (modules/welcome, _redeemInviteThenEnter) — so an Accept link is the + * existing, exercised redemption path with nothing new behind it. What is new + * is `invite_action`, which tells the page which of the two answers was + * pressed. + * + * 🚨 ACCEPT AND DECLINE HAVE DIFFERENT AUTHENTICATION NEEDS, and the front + * end is where that is enforced, not here: + * + * accept requires a session. Membership is granted to an ACCOUNT, so + * there has to be one; the welcome route signs them in (or up) + * first and redeems afterwards, which is what it already did. + * decline requires none. Holding the secret is the authorisation, and + * hub.decline_invite is scoped accordingly. Demanding a sign-in to + * say no would mean the one answer that needs no account could + * only be given by creating one. + * + * `hub_id` and `name` ride along for the same reason the CTA carries them — + * the desk can name and open the workspace once the answer is in — and grant + * nothing on their own. + * + * @param {string} token the invitation's secret + * @param {string} hub_id the workspace being invited to + * @param {string} [hubname] workspace display name, for the page's copy + * @param {string} action "accept" | "decline" + * @returns {string} absolute URL + */ + _inviteAnswerLink(token, hub_id, hubname, action) { + const base = `${this._endpointBase()}/#/welcome/signin`; + const q = [ + `invite=${encodeURIComponent(token)}`, + `invite_action=${encodeURIComponent(action)}`, + ]; + if (hub_id) q.push(`hub_id=${encodeURIComponent(hub_id)}`); + if (hubname) q.push(`name=${encodeURIComponent(hubname)}`); + return `${base}?${q.join("&")}`; + } + /** * Canonical app base for links mailed to people who are NOT yet in a workspace: * `https://`, the same shape analytics-server's @@ -711,7 +753,18 @@ class __private_hub extends Hub { fullname, from_fullname: fullname, message: meta.message || null, - privilege: meta.privilege || null + privilege: meta.privilege || null, + // WHAT MAKES THE ROW ANSWERABLE. Present only on rows written by + // _notifyInvitee — a real invitation — and absent on the receipt + // _grantMembership writes when an admin adds somebody directly + // (add_contributors), which is not an invitation and has nothing to + // accept. The client keys its Accept/Decline buttons on it, so an + // older row simply renders as it always did. + // + // Safe to hand over: it is this user's own invitation, the same secret + // their email already carries, and notification_hub_invites is scoped + // to the recipient. + invite_token: meta.token || null }; }); this.output.list(out); @@ -961,17 +1014,26 @@ class __private_hub extends Hub { /** * Record that a workspace invitation was SENT, for the Viral loop page. * - * THIS IS THE ONLY RECORD OF THE INVITATION for the branch where the invitee - * already has an account. That branch grants membership on the spot and - * writes nothing else — no token, no pending_invitation row, not even the - * writeAudit its sibling branch writes — so before this call the most common - * in-org invitation left no trace anywhere and "invite rate" was unanswerable. + * 🚨 TWO PROCEDURES, AND THE CALLER MUST SAY WHICH. They differ on one rule: + * what had_account = 1 implies. + * + * invite_track_mark stamps accept_time = sent_time for an existing + * account, because the caller GRANTED membership on + * the spot — the invitation was accepted by + * construction and no later answer is coming. + * invite_track_mark_v2 leaves accept_time NULL, because the caller SENT an + * invitation the recipient must answer. + * + * `answerable` picks between them, and it DEFAULTS TO THE OLD ONE on purpose. + * invite() opts in; invite_with_roles still grants immediately and must keep + * the old semantics, or every one of its grants would be recorded as an + * invitation nobody ever answered. A default that silently changed under it + * is exactly the failure mode the _v2 split exists to prevent, and it would + * be invisible — same arity, no error, wrong number. * - * had_account IS NOT COSMETIC. An invitation to an existing account is - * accepted by construction (the grant already happened), so invite_track - * stamps accept_time = sent_time for it. Blending those into one accept rate - * reports a figure near 100% that says nothing about whether invitations - * persuade anyone, which is why the flag is stored and the page shows both. + * had_account IS STILL NOT COSMETIC: it separates invitations that had to + * talk somebody into opening an account from ones that only had to be said + * yes to, and viral_loop reports both rates. * * NEVER THROWS, for the same reason as _trackWorkspaceMembers: a failed * counter must not fail an invitation that actually went out. @@ -980,14 +1042,17 @@ class __private_hub extends Hub { * @param {string} o.hub_id workspace invited into * @param {string} o.email address invited * @param {string} [o.invitee_uid] set when the invitee already has an account - * @param {boolean} o.had_account true when membership was granted immediately + * @param {boolean} o.had_account true when the invitee already had an account * @param {string} o.source which call site wrote it + * @param {boolean} [o.answerable] true when the recipient must accept before + * becoming a member — routes to invite_track_mark_v2. Default false keeps + * the instant-grant semantics for callers that still grant. */ - async _trackInviteSent({ hub_id, email, invitee_uid, had_account, source }) { + async _trackInviteSent({ hub_id, email, invitee_uid, had_account, source, answerable = false }) { if (!hub_id || !email) return; try { await this.yp.await_proc( - "invite_track_mark", + answerable ? "invite_track_mark_v2" : "invite_track_mark", this.uid, hub_id, email, invitee_uid || null, had_account ? 1 : 0, source || "hub_invite" ); @@ -998,6 +1063,23 @@ class __private_hub extends Hub { } } + /** + * Write membership: add_member, the permission grants, the audit line, the + * "you are in" notification and the member-count rollup. + * + * 🚨 NOT REACHED BY hub.invite ANY MORE. Inviting mints an invitation and + * stops; membership is written when the person accepts, by accept_invite, + * which does its own add_member with the seat and over-limit guards an + * answered invitation needs. The remaining callers are the paths that add + * somebody DIRECTLY, without asking: add_contributors (an admin picking an + * existing contact) and the dead invite_with_roles. + * + * The `hub_invite_received` row it writes therefore means what it always + * meant HERE — a receipt for a membership that now exists — and not the + * answerable invitation _notifyInvitee writes. That one carries a token; this + * one does not, which is what the notification row keys its Accept/Decline + * buttons on. + */ async _grantMembership(uid, privilege, expiry, message, mfs_home, hub_name, from_fullname) { const r = await this.db.await_proc("add_member", uid, privilege, expiry); if (!r || !r.db_name) return null; @@ -1043,12 +1125,9 @@ class __private_hub extends Hub { } // Notify online members (admins with the Folder settings permission matrix // open) so the new member appears immediately without a manual reload. - // Covers both callers of _grantMembership: invite() branch B (drumate - // already exists) and add_contributors(). await notifyMemberJoined(this, this.hub.get(Attr.id), uid); - // Single choke point for granting, so this one call covers invite()'s - // existing-account branch AND add_contributors() — no second place to - // forget when another caller is added later. + // Single choke point for granting, so no second place to forget when + // another caller is added later. await this._trackWorkspaceMembers(this.hub.get(Attr.id)); return r; } @@ -1447,109 +1526,115 @@ class __private_hub extends Hub { if (isArray(drumate)) drumate = drumate[0]; const isDrumate = drumate && drumate.id; - // --- Functional work, still keyed on account status (NOT the email) --- + // --- ONE INVITATION, WHATEVER THE ACCOUNT STATUS --- + // + // 🚨 AN EXISTING ACCOUNT USED TO BE ADDED TO THE WORKSPACE RIGHT HERE, + // by _grantMembership, before the email had even been composed. The + // recipient was never asked: the first they knew of it was a workspace + // appearing in their sidebar, and the email that followed announced + // something already done. There was no accept and no decline because + // there was nothing left to answer. + // + // Both branches now MINT AN INVITATION and stop. Membership is written + // by hub.accept_invite, when the person says yes — from the email's + // Accept link or from the Accept button on the notification row. + // + // What is left of the account-status branch is one thing only: whether + // there is a Drumee user to notify. Everything else — the token, the + // pending row, the audit line, the tracking, the email — is identical + // for both, which is the point. Two half-shaped invitations that had to + // be told apart everywhere downstream are now one. + // + // THE TOKEN IS THE INVITATION. It is what Accept redeems and what + // Decline closes, and its secret has to reach the recipient: by email + // for everyone, and additionally inside the notification row for + // somebody who already has an account. + const token = await this._addInviteToken(email, hubId, privilege, expiryTs); + // THE PENDING ROW IS WHAT SIGNUP GRANTS FROM. create_account calls + // _resolve_pending_invitation(email), which reads + // pending_invitation_get_by_email and adds the new account to each hub; + // nothing redeems the TOKEN during sign-up. So an invitation that + // writes only a token leaves a newcomer with no membership at all — + // that was the share-workspace bug, where the invitee signed up and + // landed on a desk showing only the three default workspaces. + // + // Written for an existing account too, and harmlessly: that row is only + // ever read at account creation, which has already happened for them. + // It is also what makes them visible to the admin console's Pending + // Invites, which was blind to this branch while it granted on the spot. + // hub.decline_invite and hub.accept_invite both delete it + // (pending_invitation_delete), so a refusal cannot be undone by signing + // up afterwards. + await this.yp.await_proc( + "yp_add_pending_invitation", hubId, 0, privilege, email + ); + pending = true; + // Now written for BOTH branches. The existing-account branch left no + // audit line at all while it granted instead of inviting, so an + // administrator reading the workspace's log saw only 'added' with no + // invitation before it. + await writeAudit(this, { + db: this.hub.get(Attr.db_name), + uid: this.uid, + action: 'invite_sent', + category: 'member', + notify_to: 'admin', + entity_id: hubId, + log: `Invite sent to ${email} for workspace '${hubname}'`, + }); + // _v2, and the version is the whole point: the old procedure stamps + // accept_time = sent_time whenever had_account = 1, because its caller + // granted membership itself. Nothing is granted here, so an invitation + // to an existing account is as unanswered as any other until it is + // accepted. See invite_track_mark_v2. + await this._trackInviteSent({ + hub_id: hubId, + email, + invitee_uid: isDrumate ? drumate.id : null, + had_account: !!isDrumate, + source: "hub_invite", + answerable: true, + }); if (isDrumate) { - // Existing account: grant membership now and push it over the socket, so - // the workspace shows up in a live session without a reload. - const r = await this._grantMembership(drumate.id, privilege, 0, message, mfs_home, hubname, username); - // `r`, not "it did not throw": _grantMembership returns null when - // add_member hands back no row, and it bails BEFORE permission_grant - // in that case — so a null there means no membership was written. - granted = !!r; - // The ONLY record this branch leaves of the invitation. Everything - // else here is the grant, not the invite: no token, no pending row, - // and the writeAudit below belongs to the other branch. Tracked - // before the socket push so a WS failure cannot lose the row. - await this._trackInviteSent({ + await this._notifyInvitee(drumate.id, { hub_id: hubId, - email, - invitee_uid: drumate.id, - had_account: true, - source: "hub_invite", + hub_name: hubname, + message, + from_fullname: username, + privilege, + token, }); - if (r) { - try { - const hub = await this.yp.await_proc( - `${r.db_name}.mfs_access_node`, drumate.id, hubId - ); - if (hub) { - hub.message = message; - hub.ownpath = '/'; - hub.hub_id = hub.actual_hub_id; - hub.db_name = hub.actual_db; - const sockets = await this.yp.await_proc('user_sockets', drumate.id); - await RedisStore.sendData(this.payload(hub, { service: "hub.invite_received" }), sockets); - await RedisStore.sendData(this.payload(hub, { service: "hub.add_contributors" }), sockets); - } - } catch (err) { - this.warn("[hub] invite: ws notify failed for", drumate.id, err && err.message); - } - } - } else { - // No account yet: mint an invite token so the address can be redeemed - // after sign-up, AND record a pending invitation so the membership is - // actually granted when the account appears. - // - // The pending row is what does the granting: signup's create_account - // calls _resolve_pending_invitation(email), which reads - // pending_invitation_get_by_email and adds the new user to each hub. - // Nothing anywhere redeems the invite TOKEN during sign-up — it is for - // the link flow — so an invite that writes only a token leaves the - // person with no membership at all. - // - // This used to be gated on `!isShareLink`, which excluded exactly the - // external (area === "share") workspaces: an invitee with no account - // signed up, was never added, and landed on a desk showing only the - // three default workspaces. Opening the workspace they were invited to - // then failed with "the file you requested does not exist", which is - // what a hub with no grant looks like from the client. - // - // Internal is unaffected: it already took the branch that writes this - // row, and it still writes exactly the same row. - await this._addInviteToken(email, hubId, privilege, expiryTs); - await this.yp.await_proc( - "yp_add_pending_invitation", hubId, 0, privilege, email - ); - await writeAudit(this, { - db: this.hub.get(Attr.db_name), - uid: this.uid, - action: 'invite_sent', - category: 'member', - notify_to: 'admin', - entity_id: hubId, - log: `Invite sent to ${email} for workspace '${hubname}'`, - }); - // Recoverable from yp.token even without this (the backfill does - // exactly that), but recorded live so the two branches produce one - // uniform row shape and the page never has to special-case which - // half of an invitation it is looking at. accept_time stays NULL - // until signup redeems the pending row — see invite_track_accept. - await this._trackInviteSent({ - hub_id: hubId, - email, - had_account: false, - source: "hub_invite", - }); - pending = true; } // --- One email for everyone, varying only by workspace scope --- + // + // THE SUBJECT NO LONGER CLAIMS THE DEED IS DONE. "added you to" was + // literally true while this granted membership on the spot; it is a + // lie now, and the one line of the email most people read. await this._sendInviteEmail( WORKSPACE_INVITE_TPL, email, workspace_external - ? `${username} shared ${hubname} with you` - : `${username} added you to ${hubname}`, + ? `${username} invited you to ${hubname}` + : `${username} invited you to join ${hubname}`, { inviter_name: username, workspace_name: hubname, + // Kept, and still the target of the workspace preview's own links, + // so an email opened by somebody who has already answered is not a + // dead end. link: ctaLink, + // The two answers. Both carry the token and nothing else identifying + // — holding the secret IS the authorisation, exactly as it already + // was for redemption. + accept_link: this._inviteAnswerLink(token, hubId, hubname, "accept"), + decline_link: this._inviteAnswerLink(token, hubId, hubname, "decline"), workspace_external, preview_items, recent_messages, }, ); - results.push({ email, status: "ok", granted, pending }); + results.push({ email, status: "ok", granted, pending, had_account: !!isDrumate }); // Queue for address-book bookkeeping — only invitees whose branch // actually succeeded (a failure throws above and must leave no contact). // Deliberately deferred until every invite is done, see below. @@ -1589,9 +1674,18 @@ class __private_hub extends Hub { } /** - * Mint a hub_invite token for an address with no Drumee account yet, so the - * invite can be redeemed after sign-up (accept_invite). Token only — the email - * is sent once by invite(), the same one every invitee gets. + * Mint a hub_invite token: the invitation itself, and the only thing that can + * redeem or refuse one. Every invitee gets one now, not just an address with + * no Drumee account — see invite(). + * + * THE SECRET IS RETURNED, and callers need it. It is what the email's Accept + * and Decline links carry, and what the notification row hands back to + * hub.accept_invite / hub.decline_invite. Before invitations could be + * answered this only had to exist, so nothing read it. + * + * REPLACE semantics live in token_hub_invite_add: re-inviting the same + * address to the same workspace as the same inviter supersedes the previous + * token, so a refusal does not block a later invitation. */ async _addInviteToken(email, hubId, privilege, expiryTs) { const { randomBytes } = require("crypto"); @@ -1601,6 +1695,63 @@ class __private_hub extends Hub { await this.yp.await_proc( "token_hub_invite_add", email, "", secret, method, this.uid, metadata, expiryTs ); + return secret; + } + + /** + * Tell an invitee who already has an account that they have been invited. + * + * THE NOTIFICATION IS THE INVITATION on this branch, not a receipt for + * something already done. It used to be written by _grantMembership, after + * the membership had been created — "you are in" — and the row carried no way + * to answer because there was nothing to answer. It now carries the token, so + * the row can offer Accept and Decline (activity item skeleton reads + * `invite_token`). + * + * 🚨 `hub.add_contributors` IS NOT PUSHED HERE, and its absence is the + * feature. That is the client's "you are now a member" signal — window/utils + * newContent() adds the workspace tile on it — and pushing it for somebody who + * has not accepted would put the workspace on their desk, which is exactly + * the auto-join being removed. Only `hub.invite_received` goes out: the + * activity panel refreshes its feed on it, which is how the invitation + * appears without a reload. + * + * NEVER THROWS. A notification is not the invitation's record — the token and + * the pending row are — so a socket that is not there, or a contact_activity + * write that fails, must not fail an invitation whose email is about to go + * out. The invitee still gets the email, and the row is rebuilt from + * contact_activity on their next feed read. + */ + async _notifyInvitee(uid, { hub_id, hub_name, message, from_fullname, privilege, token }) { + try { + await this.yp.await_proc( + "contact_log_activity", this.uid, uid, "hub_invite_received", + { + hub_id, + hub_name, + message, + from_fullname, + privilege, + // Read back out by activity.mapHubInviteRow and + // hub.invite_received_get, both of which surface it as + // `invite_token`. Safe to hand to this user: it is their own + // invitation, the same secret their email already carries, and the + // feed procs are scoped to the recipient. + token, + } + ); + } catch (err) { + this.warn("[hub] invite: activity row failed for", uid, err && err.message); + } + try { + const sockets = await this.yp.await_proc('user_sockets', uid); + await RedisStore.sendData( + this.payload({ hub_id, hub_name }, { service: "hub.invite_received" }), + sockets + ); + } catch (err) { + this.warn("[hub] invite: ws notify failed for", uid, err && err.message); + } } /** @@ -1669,6 +1820,11 @@ class __private_hub extends Hub { // scope:hub/src:owner). if (held >= permission) { await this.yp.await_proc('token_hub_invite_set_status', secret, 'accepted', keep_meta); + // The invitation is ANSWERED even though nothing was granted, so it has + // to stop being pending: the panel would keep listing somebody who is + // already a member, and their bell would keep offering them buttons. + // Access itself is deliberately untouched here — see the note above. + await this._closeInvitation(hub_id, tokenRow.email, { accepted: 1 }); return this.output.data({ hub_id, already_member: 1 }); } @@ -1720,7 +1876,51 @@ class __private_hub extends Hub { `${db_name}.permission_grant`, '*', this.uid, 0, permission, 'system', 'Redeemed hub invite token' ); + // 🚨 THE CHAT STAGING GRANT, which this path has never written. + // + // A member whose role is Chat has no write bit for the workspace, by + // design, and is meant to get write on the hidden '/__chat__/__upload__' + // folder alone so an attachment can be staged before it becomes a message. + // _grantMembership writes that grant; redeeming a token did not, so anybody + // who joined by link could not attach a file to a chat — the same 403 that + // was chased and fixed on the other path (schemas 2026-09-17, + // chat_upload_grant_repair). + // + // It was survivable while token redemption was the rare branch. It is now + // how EVERY member joins, so leaving it out would reintroduce that bug as + // the normal case, on the one path nobody had looked at. + // + // Gated on CAN_CHAT exactly as _grantMembership gates it: a view-only + // member may not chat, so handing them an upload path would give them + // something their role does not carry. 'no_traversal' keeps the raised + // access on that one folder and stops user_permission letting it reach + // anything inside. + // + // mfs_home() reads DATABASE(), which inside a routine is the routine's OWN + // database — so the cross-database call resolves the INVITED workspace's + // staging folder, not the caller's. Never allowed to fail the join: a + // member without this grant has a working workspace and a chat that cannot + // attach, which is strictly better than an invitation that would not + // redeem. + if (privilegeAllows(permission, CAN_CHAT)) { + try { + const home = await this.yp.await_proc(`${db_name}.mfs_home`); + if (home && home.chat_upload_id) { + await this.yp.await_proc( + `${db_name}.permission_grant`, + home.chat_upload_id, this.uid, 0, CHAT_UPLOAD_GRANT, + 'no_traversal', 'chat upload permission' + ); + } + } catch (err) { + this.warn( + "[hub] accept_invite: chat upload grant failed for", this.uid, + err && err.message + ); + } + } await this.yp.await_proc('token_hub_invite_set_status', secret, 'accepted', keep_meta); + await this._closeInvitation(hub_id, tokenRow.email, { accepted: 1 }); await writeAudit(this, { db: db_name, uid: this.uid, @@ -1737,6 +1937,186 @@ class __private_hub extends Hub { this.output.data({ hub_id }); } + /** + * Everything an ANSWERED invitation has to stop being, whichever answer it + * got. Called by accept_invite and decline_invite so the two cannot drift + * into closing different halves of the same thing. + * + * The answer itself is recorded by the caller — the token's status is what + * says accepted or declined — and this is the cleanup around it: + * + * pending_invitation 🚨 THE ONE THAT MATTERS. That table is not a record + * of an invitation, it is the QUEUE signup grants membership from + * (_resolve_pending_invitation, in both signup.js and butler.js). Leave + * the row behind after a REFUSAL and the invitee declines, creates their + * account, and is put into the workspace they just turned down. Leave it + * after an ACCEPTANCE and signup grants a membership that already exists + * — harmless today because add_member is idempotent, but the invitation + * also goes on reading as unanswered to everything that counts that + * table, including the admin console's Pending Invites. + * + * contact_activity takes the row out of the invitee's bell. Cannot be + * left to the client: an answer given from the EMAIL knows only the + * token, never the notification's id, so the row would survive and keep + * offering Accept and Decline on a spent token. + * + * invite_track the analytics outcome, per workspace. Both + * procedures are the per-hub variants for the reason spelled out in + * invite_track_accept_hub: the per-address ones would answer somebody's + * OTHER pending invitations on the strength of this one. + * + * NEVER THROWS. The answer has already been written by the time this runs — + * membership granted, or the token marked declined — so a failure here must + * not turn a completed answer into an error the user sees and retries. Each + * step is independently guarded so one failing does not skip the rest. + * + * @param {string} hub_id the workspace answered about + * @param {string} email the address the invitation was sent to; may be + * absent on an old token, in which case only the notification is cleared + * @param {object} o + * @param {boolean} [o.accepted] true for accept, false/absent for decline + */ + async _closeInvitation(hub_id, email, { accepted = false } = {}) { + if (!hub_id) return; + if (email) { + try { + await this.yp.await_proc('pending_invitation_delete', hub_id, email); + } catch (err) { + this.warn("[hub] invitation cleanup: pending row", err && err.message); + } + } + if (this.uid) { + try { + await this.yp.await_proc( + 'contact_activity_dismiss_hub_invite', this.uid, hub_id + ); + } catch (err) { + this.warn("[hub] invitation cleanup: notification", err && err.message); + } + } + if (email) { + try { + if (accepted) { + await this.yp.await_proc( + 'invite_track_accept_hub', hub_id, email, this.uid || null + ); + } else { + await this.yp.await_proc('invite_track_decline', hub_id, email); + } + } catch (err) { + this.warn("[hub] invitation cleanup: tracking", err && err.message); + } + } + } + + /** + * Turn down a workspace invitation. + * + * 🚨 NO SESSION IS REQUIRED, and that is deliberate rather than an oversight. + * HOLDING THE SECRET IS THE AUTHORISATION, exactly as it already is for + * redemption — and this is the strictly less dangerous of the two, because + * accepting with a secret GRANTS access to a workspace while declining only + * destroys an invitation addressed to one email. + * + * Requiring a sign-in would mean the one answer that needs no account could + * only be given by creating one: an invitation is most often sent to somebody + * who has no Drumee account, and "no thanks" must not cost them a signup. + * + * WHAT IT CANNOT DO is as important as what it can. It never removes + * membership — leaving a workspace is desk.leave_hub, a different act behind + * a different permission — and token_hub_invite_decline refuses anything that + * is not an ACTIVE hub_invite token, so a stale link pressed after joining + * cannot rewrite how somebody got in. + * + * IDEMPOTENT by construction: declining twice keeps the first answer (the + * proc's own WHERE clause), and so does invite_track_decline. The second + * press reports the same `declined` rather than an error, because from the + * user's side nothing is wrong — they said no, twice. + */ + async decline_invite() { + const secret = this.input.need('token'); + const rows = await this.yp.await_proc('token_hub_invite_decline', secret); + const tokenRow = isArray(rows) ? rows[0] : rows; + // No row at all: not a hub-invite token, or one already swept away by the + // expiry cleanup in token_hub_invite_add. Nothing to answer. + if (!tokenRow || !tokenRow.secret) { + return this.output.data({ status: 'invalid' }); + } + const hub_id = tokenRow.hub_id + || String(tokenRow.method || '').slice('hub_invite:'.length); + // Already accepted — say so rather than claiming a refusal that did not + // happen. The proc left the token alone, and the person is a member. + if (tokenRow.status === 'accepted') { + return this.output.data({ status: 'already_used', hub_id }); + } + await this._closeInvitation(hub_id, tokenRow.email, { accepted: false }); + // Audited on the WORKSPACE, so its administrators can see that an + // invitation they sent was turned down — the Pending Invitations list shows + // the state, this records the moment. Best-effort: the refusal is already + // written and must not be undone by a logging failure, and the hub db may + // not resolve for a caller with no session. + try { + const db_name = await this.yp.await_func('get_db_name', hub_id); + if (db_name) { + await writeAudit(this, { + db: db_name, + uid: this.uid || null, + action: 'invite_declined', + category: 'member', + notify_to: 'admin', + entity_id: hub_id, + log: `Invite declined — ${tokenRow.email} turned down the invitation`, + }); + } + } catch (err) { + this.warn("[hub] decline_invite: audit failed", err && err.message); + } + this.output.data({ status: 'declined', hub_id }); + } + + /** + * The invitations this workspace is still waiting on, plus the ones that were + * turned down — the "Pending Invitations" section of the Access panel. + * + * 🚨 CURRENT MEMBERS ARE FILTERED OUT HERE, and they have to be filtered + * somewhere. hub_invitations reads yp, and membership lives in the + * workspace's own database, so the procedure cannot know. The usual case it + * catches: an address invited while an older invitation of its own was still + * active, then added straight to the workspace by add_contributors — which + * asks nobody and leaves the invitation open. Without this the panel would + * list the same person under Pending and under Members at once. + * + * Matched on the ADDRESS, lowercased on both sides, because an invitation has + * no uid to match on until the invitee has an account. + */ + async invitations() { + const hub_id = this.hub.get(Attr.id); + const rows = toArray(await this.yp.await_proc('hub_invitations', hub_id)); + let members = []; + try { + // Through _members_by_type, not a bare proc call: it is the one place + // that knows a PERSONAL entity's database has no hub_get_members_by_type + // at all, and that calling it there raises ER_SP_DOES_NOT_EXIST and tears + // down this request's DB connection. + members = toArray(await this._members_by_type('all', 1)); + } catch (err) { + // A member list we could not read is not a reason to hide the + // invitations: showing one row too many is recoverable by the admin, + // showing nothing looks like the invitations were never sent. + this.warn("[hub] invitations: member list failed", err && err.message); + } + const seated = new Set( + members + .map((m) => String((m && m.email) || "").trim().toLowerCase()) + .filter(Boolean) + ); + this.output.list( + rows.filter( + (r) => !seated.has(String(r.email || "").trim().toLowerCase()) + ) + ); + } + /** * Top-3 workspace items (folders first, then files) for the invite email * previews. Listing is read from the inviter's perspective (this.uid) since a @@ -1816,15 +2196,22 @@ class __private_hub extends Hub { /** * WHAT TO TELL THE CALLER when one invitee's turn threw. * - * The loop above is ordered grant-then-notify, so where the throw happened + * The loop above is ordered mint-then-notify, so where the throw happened * decides what is true afterwards: * - * - after _grantMembership → the person IS a member. Access is live, the - * `hub.member_joined` push has already gone out, and only the email is - * missing. - * - after the pending row → nothing is granted yet, but the invitation is - * recorded and signup will honour it. Only the email is missing. - * - before either → nothing happened. + * - after the pending row → the invitation EXISTS. It is in the Pending + * Invitations list, and an invitee who already has an account can still + * accept it from their notification row even if the email never left. + * - before it → nothing happened. + * + * 🚨 `granted` IS ALWAYS FALSE NOW and the branch that read it is dead code + * kept deliberately. Nothing in invite() grants membership any more — that + * moved to hub.accept_invite — so no throw here can leave somebody a member. + * The parameter stays because the result shape is part of this service's + * contract with three callers (the Access panel, the rail's Invite popup and + * the folder window), and because reviving the grant behind this function's + * back would otherwise report "nothing was granted" while the person had + * access. * * Reporting all three as a bare "the MTA rejected
" is wrong in the * direction that costs the admin the most: they read it as "the invite did @@ -1852,10 +2239,10 @@ class __private_hub extends Hub { + `to ${ws} and can open it now.`; } if (pending) { - return `${why}. The invitation to ${ws} is recorded and will be honoured ` - + `when ${email} signs up.`; + return `${why}. The invitation to ${ws} is recorded — ${email} can still ` + + `accept it, and it will be honoured when they sign up.`; } - return `${why}. Nothing was granted — ${email} has no access to ${ws}.`; + return `${why}. Nothing was invited — ${email} has no invitation to ${ws}.`; } /** diff --git a/service/private/templates/butler/workspace-invite-member.html b/service/private/templates/butler/workspace-invite-member.html index fffc988..24d39af 100644 --- a/service/private/templates/butler/workspace-invite-member.html +++ b/service/private/templates/butler/workspace-invite-member.html @@ -11,7 +11,7 @@ - <% if (_external) { %><%= inviter_name %> shared <%= workspace_name %> with you<% } else { %><%= inviter_name %> added you to <%= workspace_name %><% } %> + <% if (_external) { %><%= inviter_name %> invited you to <%= workspace_name %><% } else { %><%= inviter_name %> invited you to join <%= workspace_name %><% } %> @@ -34,17 +34,29 @@

+ <% if (_external) { %> - <%= inviter_name %> shared <%= workspace_name %> with you + <%= inviter_name %> invited you to <%= workspace_name %> <% } else { %> - <%= inviter_name %> added you to <%= workspace_name %> + <%= inviter_name %> invited you to join <%= workspace_name %> <% } %>

<% if (_external) { %> - You've been given access to the <%= workspace_name %> shared workspace on Drumee. Open it to view the shared files and join the conversation. + You've been invited to the <%= workspace_name %> shared workspace on Drumee. Accept to view the shared files and join the conversation. <% } else { %> - You now have access to the <%= workspace_name %> internal workspace on Drumee. It stays private to its members — open it to view the files and join the conversation. + You've been invited to the <%= workspace_name %> internal workspace on Drumee. It stays private to its members — accept to view the files and join the conversation. <% } %>

@@ -238,20 +250,50 @@

 
- + - + +
- - - Join Drumee to view this folder → + + + Accept invitation + +   + + Decline
+ +
 
+ + +

+ This invitation expires in 7 days. You will not join <%= workspace_name %> unless you accept. +

+ @@ -277,7 +319,7 @@

DRUMEE WORKSPACE

-<% if (_external) { %>You received this because <%= inviter_name %> shared <%= workspace_name %> with you. Manage your email preferences in your account settings.<% } else { %>You received this because <%= inviter_name %> added you to <%= workspace_name %>, an internal workspace. Manage your email preferences in your account settings.<% } %> +<% if (_external) { %>You received this because <%= inviter_name %> invited you to <%= workspace_name %>. Manage your email preferences in your account settings.<% } else { %>You received this because <%= inviter_name %> invited you to <%= workspace_name %>, an internal workspace. Manage your email preferences in your account settings.<% } %>
From 52e7f8ea2d4a4656dd90343788727399fb5d5960 Mon Sep 17 00:00:00 2001 From: "phamtobao@gmail.com" Date: Tue, 22 Sep 2026 23:59:07 +0400 Subject: [PATCH 04/34] fix(hub): send invite emails in batches of five instead of one awaited send each --- service/private/hub.js | 168 +++++++++++++++++++++++++++++------------ 1 file changed, 118 insertions(+), 50 deletions(-) diff --git a/service/private/hub.js b/service/private/hub.js index 24563ef..09ec162 100644 --- a/service/private/hub.js +++ b/service/private/hub.js @@ -56,6 +56,11 @@ const EXTERNAL_AREAS = ["share", "dmz"]; // account; only the body copy varies (internal vs external workspace). // Replaces the former hub-invite-added / hub-invite-link / hub-invite-signup trio. const WORKSPACE_INVITE_TPL = "workspace-invite-member"; +// Invite emails go out this many at a time. One SMTP round-trip to the relay +// costs ~4.5 s (a fresh connection per message), so sending them one after +// another held a 30-address invite for over two minutes; five in flight keeps +// the relay's per-IP connection cap comfortable while dividing that by five. +const INVITE_MAIL_BATCH = 5; /** * True when a workspace area is shared outside the member circle. @@ -1433,9 +1438,14 @@ class __private_hub extends Hub { // list, and a caller may repeat an address; remember each person once. const remembered = new Set(); const toRemember = []; - const results = []; - - for (const email of invitees) { + // Indexed by position so the reply lists invitees in the order they were + // typed, even though the email step below runs in batches. + const results = new Array(invitees.length); + // Everyone whose membership/pending work succeeded, waiting for the mail + // step. The DB work is milliseconds per address; the mail is not. + const mailQueue = []; + + for (const [idx, email] of invitees.entries()) { // WHAT ACTUALLY LANDED before the email step, so a mail failure can say // so rather than reading as "nothing happened" — see _inviteFailureReason // and the catch at the bottom of this loop. Reset per invitee: one bad @@ -1533,34 +1543,11 @@ class __private_hub extends Hub { pending = true; } - // --- One email for everyone, varying only by workspace scope --- - await this._sendInviteEmail( - WORKSPACE_INVITE_TPL, - email, - workspace_external - ? `${username} shared ${hubname} with you` - : `${username} added you to ${hubname}`, - { - inviter_name: username, - workspace_name: hubname, - link: ctaLink, - workspace_external, - preview_items, - recent_messages, - }, - ); - results.push({ email, status: "ok", granted, pending }); - // Queue for address-book bookkeeping — only invitees whose branch - // actually succeeded (a failure throws above and must leave no contact). - // Deliberately deferred until every invite is done, see below. - const key = String(email).trim().toLowerCase(); - if (!remembered.has(key)) { - remembered.add(key); - toRemember.push({ email, drumate }); - } + // The email itself is sent after this loop, in batches — see below. + mailQueue.push({ idx, email, granted, pending, drumate }); } catch (err) { this.warn("[hub] invite failed for", email, err && err.message); - results.push({ + results[idx] = { email, status: "failed", // The caller needs these to know whether "failed" means "nothing @@ -1568,7 +1555,68 @@ class __private_hub extends Hub { granted, pending, reason: this._inviteFailureReason(email, hubname, granted, pending, err), - }); + }; + } + } + + // --- One email for everyone, varying only by workspace scope --- + // Batched, not one awaited send per invitee: each send is a full SMTP + // handshake with the relay (~4.5 s measured on production), and paying it + // serially made the request time grow linearly with the guest list — a + // 30-address invite held the popup for over two minutes. The per-address + // verdict is kept: Messenger reports which recipients the MTA rejected. + const subject = workspace_external + ? `${username} shared ${hubname} with you` + : `${username} added you to ${hubname}`; + const mailData = { + inviter_name: username, + workspace_name: hubname, + link: ctaLink, + workspace_external, + preview_items, + recent_messages, + }; + for (let i = 0; i < mailQueue.length; i += INVITE_MAIL_BATCH) { + const batch = mailQueue.slice(i, i + INVITE_MAIL_BATCH); + let rejected = new Set(); + let batchErr = null; + try { + rejected = await this._sendInviteEmails( + WORKSPACE_INVITE_TPL, + batch.map((b) => b.email), + subject, + mailData, + ); + } catch (err) { + // Nothing in this batch was dispatched (no MTA, unknown reply shape). + batchErr = err; + } + for (const { idx, email, granted, pending, drumate } of batch) { + const key = String(email).trim().toLowerCase(); + const err = batchErr + ? new Error(`Email delivery to ${email} failed: ${batchErr.message}`) + : rejected.has(key) + ? new Error(`Email delivery to ${email} failed: the MTA rejected ${email}`) + : null; + if (err) { + this.warn("[hub] invite failed for", email, err.message); + results[idx] = { + email, + status: "failed", + granted, + pending, + reason: this._inviteFailureReason(email, hubname, granted, pending, err), + }; + continue; + } + results[idx] = { email, status: "ok", granted, pending }; + // Queue for address-book bookkeeping — only invitees whose branch + // actually succeeded (a failure must leave no contact). + // Deliberately deferred until every invite is done, see below. + if (!remembered.has(key)) { + remembered.add(key); + toRemember.push({ email, drumate }); + } } } @@ -1585,7 +1633,7 @@ class __private_hub extends Hub { await this._rememberInvitee(email, drumate, contactBook); } - this.output.data({ results }); + this.output.data({ results: results.filter(Boolean) }); } /** @@ -1859,31 +1907,51 @@ class __private_hub extends Hub { } /** - * Gửi 1 email mời theo template app-local service/private/templates/butler/.html - */ - async _sendInviteEmail(tpl, recipient, subject, data) { + * Send the SAME invite email (service/private/templates/butler/.html) + * to a batch of addresses at once. + * + * Messenger.send accepts an array of recipients, posts them concurrently on + * one transport and answers `{ recipient, error }`, where `error` lists the + * addresses whose sendMail rejected (null when every one was delivered). + * + * AWAITED, and the reply judged by SHAPE rather than by a truthy `.error` + * — see service/lib/mail-result for why that distinction is the whole bug. + * dispatch() returns undefined before the mail reaches an MTA, and send() + * returns the rendered HTML when no transport is configured; both read as + * success under an `.error` test, which is how an invite came to report + * status:"ok" while nothing ever left the box. + * + * @param {string} tpl template name under templates/butler + * @param {string[]} recipients addresses for this batch + * @param {string} subject + * @param {Object} data template data, the same for every recipient + * @returns {Promise>} addresses (lowercased) the MTA rejected + * @throws {Error} when nothing in the batch was dispatched at all + */ + async _sendInviteEmails(tpl, recipients, subject, data) { const tplPath = resolve(__dirname, "templates", "butler", `${tpl}.html`); - const msg = new Messenger({ subject, recipient, handler: this.exception.email }); + const msg = new Messenger({ + subject, + recipient: recipients, + handler: this.exception.email, + }); const html = msg.renderFrom(tplPath, data); // Display-name From ("Drumee" ) so the inbox shows // "Drumee", matching the contact-add emails. const from = butlerFrom(); - // AWAITED, and the result judged by SHAPE rather than by a truthy `.error` - // — see service/lib/mail-result for why that distinction is the whole bug. - // dispatch() returns undefined before the mail reaches an MTA, and send() - // returns the rendered HTML when no transport is configured; both read as - // success under an `.error` test, which is how an invite came to report - // status:"ok" while nothing ever left the box. - // - // Awaiting costs one SMTP round-trip per invitee. That is the latency the - // non-blocking dispatch() was introduced to avoid, and it is the price of - // being able to tell the caller anything true at all. It also holds one - // conversation open at a time, which the relay's per-IP connection cap - // wants regardless. - const reason = mailFailure(await msg.send({ html, from })); - if (reason) { - throw new Error(`Email delivery to ${recipient} failed: ${reason}`); + const result = await msg.send({ html, from }); + const perRecipient = !!(result && Array.isArray(result.error)); + const reason = mailFailure(result); + if (reason && !perRecipient) { + throw new Error(reason); + } + const rejected = new Set(); + if (perRecipient) { + for (const r of result.error) { + rejected.add(String(r).trim().toLowerCase()); + } } + return rejected; } /** From c4055b6536c204ea92d89052007714efa2ee9812 Mon Sep 17 00:00:00 2001 From: "phamtobao@gmail.com" Date: Wed, 23 Sep 2026 00:54:40 +0400 Subject: [PATCH 05/34] fix(hub): send invite emails 25 at a time, the SMTP session cost dominates --- service/private/hub.js | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/service/private/hub.js b/service/private/hub.js index 09ec162..03fff0a 100644 --- a/service/private/hub.js +++ b/service/private/hub.js @@ -56,11 +56,15 @@ const EXTERNAL_AREAS = ["share", "dmz"]; // account; only the body copy varies (internal vs external workspace). // Replaces the former hub-invite-added / hub-invite-link / hub-invite-signup trio. const WORKSPACE_INVITE_TPL = "workspace-invite-member"; -// Invite emails go out this many at a time. One SMTP round-trip to the relay -// costs ~4.5 s (a fresh connection per message), so sending them one after -// another held a 30-address invite for over two minutes; five in flight keeps -// the relay's per-IP connection cap comfortable while dividing that by five. -const INVITE_MAIL_BATCH = 5; +// Invite emails go out this many at a time. Each message opens its own SMTP +// session to the relay, and the session costs ~4 s before a byte of mail is +// sent (connect + STARTTLS 1.8 s, AUTH 2.3 s - measured 2026-09-22), against +// ~1 s for the message itself. Sessions opened together pay that once, in +// parallel: 5 at a time measured 5.8 s per batch, the same as one message, +// so a 30-address invite still took 35 s. 25 keeps two concurrent hub.invite +// calls (the popup fires one per selected workspace) under Postfix's default +// 50 connections per client IP. +const INVITE_MAIL_BATCH = 25; /** * True when a workspace area is shared outside the member circle. From 3c21a25d8c2d8333cf776b748769c257898f5a1a Mon Sep 17 00:00:00 2001 From: "phamtobao@gmail.com" Date: Tue, 22 Sep 2026 23:59:07 +0400 Subject: [PATCH 06/34] fix(hub): send invite emails in batches of five instead of one awaited send each (cherry picked from commit 52e7f8ea2d4a4656dd90343788727399fb5d5960) --- service/private/hub.js | 168 +++++++++++++++++++++++++++++------------ 1 file changed, 118 insertions(+), 50 deletions(-) diff --git a/service/private/hub.js b/service/private/hub.js index 24563ef..09ec162 100644 --- a/service/private/hub.js +++ b/service/private/hub.js @@ -56,6 +56,11 @@ const EXTERNAL_AREAS = ["share", "dmz"]; // account; only the body copy varies (internal vs external workspace). // Replaces the former hub-invite-added / hub-invite-link / hub-invite-signup trio. const WORKSPACE_INVITE_TPL = "workspace-invite-member"; +// Invite emails go out this many at a time. One SMTP round-trip to the relay +// costs ~4.5 s (a fresh connection per message), so sending them one after +// another held a 30-address invite for over two minutes; five in flight keeps +// the relay's per-IP connection cap comfortable while dividing that by five. +const INVITE_MAIL_BATCH = 5; /** * True when a workspace area is shared outside the member circle. @@ -1433,9 +1438,14 @@ class __private_hub extends Hub { // list, and a caller may repeat an address; remember each person once. const remembered = new Set(); const toRemember = []; - const results = []; - - for (const email of invitees) { + // Indexed by position so the reply lists invitees in the order they were + // typed, even though the email step below runs in batches. + const results = new Array(invitees.length); + // Everyone whose membership/pending work succeeded, waiting for the mail + // step. The DB work is milliseconds per address; the mail is not. + const mailQueue = []; + + for (const [idx, email] of invitees.entries()) { // WHAT ACTUALLY LANDED before the email step, so a mail failure can say // so rather than reading as "nothing happened" — see _inviteFailureReason // and the catch at the bottom of this loop. Reset per invitee: one bad @@ -1533,34 +1543,11 @@ class __private_hub extends Hub { pending = true; } - // --- One email for everyone, varying only by workspace scope --- - await this._sendInviteEmail( - WORKSPACE_INVITE_TPL, - email, - workspace_external - ? `${username} shared ${hubname} with you` - : `${username} added you to ${hubname}`, - { - inviter_name: username, - workspace_name: hubname, - link: ctaLink, - workspace_external, - preview_items, - recent_messages, - }, - ); - results.push({ email, status: "ok", granted, pending }); - // Queue for address-book bookkeeping — only invitees whose branch - // actually succeeded (a failure throws above and must leave no contact). - // Deliberately deferred until every invite is done, see below. - const key = String(email).trim().toLowerCase(); - if (!remembered.has(key)) { - remembered.add(key); - toRemember.push({ email, drumate }); - } + // The email itself is sent after this loop, in batches — see below. + mailQueue.push({ idx, email, granted, pending, drumate }); } catch (err) { this.warn("[hub] invite failed for", email, err && err.message); - results.push({ + results[idx] = { email, status: "failed", // The caller needs these to know whether "failed" means "nothing @@ -1568,7 +1555,68 @@ class __private_hub extends Hub { granted, pending, reason: this._inviteFailureReason(email, hubname, granted, pending, err), - }); + }; + } + } + + // --- One email for everyone, varying only by workspace scope --- + // Batched, not one awaited send per invitee: each send is a full SMTP + // handshake with the relay (~4.5 s measured on production), and paying it + // serially made the request time grow linearly with the guest list — a + // 30-address invite held the popup for over two minutes. The per-address + // verdict is kept: Messenger reports which recipients the MTA rejected. + const subject = workspace_external + ? `${username} shared ${hubname} with you` + : `${username} added you to ${hubname}`; + const mailData = { + inviter_name: username, + workspace_name: hubname, + link: ctaLink, + workspace_external, + preview_items, + recent_messages, + }; + for (let i = 0; i < mailQueue.length; i += INVITE_MAIL_BATCH) { + const batch = mailQueue.slice(i, i + INVITE_MAIL_BATCH); + let rejected = new Set(); + let batchErr = null; + try { + rejected = await this._sendInviteEmails( + WORKSPACE_INVITE_TPL, + batch.map((b) => b.email), + subject, + mailData, + ); + } catch (err) { + // Nothing in this batch was dispatched (no MTA, unknown reply shape). + batchErr = err; + } + for (const { idx, email, granted, pending, drumate } of batch) { + const key = String(email).trim().toLowerCase(); + const err = batchErr + ? new Error(`Email delivery to ${email} failed: ${batchErr.message}`) + : rejected.has(key) + ? new Error(`Email delivery to ${email} failed: the MTA rejected ${email}`) + : null; + if (err) { + this.warn("[hub] invite failed for", email, err.message); + results[idx] = { + email, + status: "failed", + granted, + pending, + reason: this._inviteFailureReason(email, hubname, granted, pending, err), + }; + continue; + } + results[idx] = { email, status: "ok", granted, pending }; + // Queue for address-book bookkeeping — only invitees whose branch + // actually succeeded (a failure must leave no contact). + // Deliberately deferred until every invite is done, see below. + if (!remembered.has(key)) { + remembered.add(key); + toRemember.push({ email, drumate }); + } } } @@ -1585,7 +1633,7 @@ class __private_hub extends Hub { await this._rememberInvitee(email, drumate, contactBook); } - this.output.data({ results }); + this.output.data({ results: results.filter(Boolean) }); } /** @@ -1859,31 +1907,51 @@ class __private_hub extends Hub { } /** - * Gửi 1 email mời theo template app-local service/private/templates/butler/.html - */ - async _sendInviteEmail(tpl, recipient, subject, data) { + * Send the SAME invite email (service/private/templates/butler/.html) + * to a batch of addresses at once. + * + * Messenger.send accepts an array of recipients, posts them concurrently on + * one transport and answers `{ recipient, error }`, where `error` lists the + * addresses whose sendMail rejected (null when every one was delivered). + * + * AWAITED, and the reply judged by SHAPE rather than by a truthy `.error` + * — see service/lib/mail-result for why that distinction is the whole bug. + * dispatch() returns undefined before the mail reaches an MTA, and send() + * returns the rendered HTML when no transport is configured; both read as + * success under an `.error` test, which is how an invite came to report + * status:"ok" while nothing ever left the box. + * + * @param {string} tpl template name under templates/butler + * @param {string[]} recipients addresses for this batch + * @param {string} subject + * @param {Object} data template data, the same for every recipient + * @returns {Promise>} addresses (lowercased) the MTA rejected + * @throws {Error} when nothing in the batch was dispatched at all + */ + async _sendInviteEmails(tpl, recipients, subject, data) { const tplPath = resolve(__dirname, "templates", "butler", `${tpl}.html`); - const msg = new Messenger({ subject, recipient, handler: this.exception.email }); + const msg = new Messenger({ + subject, + recipient: recipients, + handler: this.exception.email, + }); const html = msg.renderFrom(tplPath, data); // Display-name From ("Drumee" ) so the inbox shows // "Drumee", matching the contact-add emails. const from = butlerFrom(); - // AWAITED, and the result judged by SHAPE rather than by a truthy `.error` - // — see service/lib/mail-result for why that distinction is the whole bug. - // dispatch() returns undefined before the mail reaches an MTA, and send() - // returns the rendered HTML when no transport is configured; both read as - // success under an `.error` test, which is how an invite came to report - // status:"ok" while nothing ever left the box. - // - // Awaiting costs one SMTP round-trip per invitee. That is the latency the - // non-blocking dispatch() was introduced to avoid, and it is the price of - // being able to tell the caller anything true at all. It also holds one - // conversation open at a time, which the relay's per-IP connection cap - // wants regardless. - const reason = mailFailure(await msg.send({ html, from })); - if (reason) { - throw new Error(`Email delivery to ${recipient} failed: ${reason}`); + const result = await msg.send({ html, from }); + const perRecipient = !!(result && Array.isArray(result.error)); + const reason = mailFailure(result); + if (reason && !perRecipient) { + throw new Error(reason); + } + const rejected = new Set(); + if (perRecipient) { + for (const r of result.error) { + rejected.add(String(r).trim().toLowerCase()); + } } + return rejected; } /** From 9f33fe7629e151a0e7da1888b20bea77f79a6d3c Mon Sep 17 00:00:00 2001 From: "phamtobao@gmail.com" Date: Wed, 23 Sep 2026 00:54:40 +0400 Subject: [PATCH 07/34] fix(hub): send invite emails 25 at a time, the SMTP session cost dominates (cherry picked from commit c4055b6536c204ea92d89052007714efa2ee9812) --- service/private/hub.js | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/service/private/hub.js b/service/private/hub.js index 09ec162..03fff0a 100644 --- a/service/private/hub.js +++ b/service/private/hub.js @@ -56,11 +56,15 @@ const EXTERNAL_AREAS = ["share", "dmz"]; // account; only the body copy varies (internal vs external workspace). // Replaces the former hub-invite-added / hub-invite-link / hub-invite-signup trio. const WORKSPACE_INVITE_TPL = "workspace-invite-member"; -// Invite emails go out this many at a time. One SMTP round-trip to the relay -// costs ~4.5 s (a fresh connection per message), so sending them one after -// another held a 30-address invite for over two minutes; five in flight keeps -// the relay's per-IP connection cap comfortable while dividing that by five. -const INVITE_MAIL_BATCH = 5; +// Invite emails go out this many at a time. Each message opens its own SMTP +// session to the relay, and the session costs ~4 s before a byte of mail is +// sent (connect + STARTTLS 1.8 s, AUTH 2.3 s - measured 2026-09-22), against +// ~1 s for the message itself. Sessions opened together pay that once, in +// parallel: 5 at a time measured 5.8 s per batch, the same as one message, +// so a 30-address invite still took 35 s. 25 keeps two concurrent hub.invite +// calls (the popup fires one per selected workspace) under Postfix's default +// 50 connections per client IP. +const INVITE_MAIL_BATCH = 25; /** * True when a workspace area is shared outside the member circle. From 2a600e1ba8992e08647ea04edbdb710610f028ee Mon Sep 17 00:00:00 2001 From: Tran Hoang Huan <121786621+tranh0anghuan@users.noreply.github.com> Date: Wed, 23 Sep 2026 09:03:09 +0700 Subject: [PATCH 08/34] feat(task): broadcast to the caller's other sessions, skip only the calling socket (#229) Co-authored-by: Drumee Dev Co-authored-by: Claude Opus 5 (1M context) --- service/private/task.js | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/service/private/task.js b/service/private/task.js index 845c30c..ecdd4e4 100644 --- a/service/private/task.js +++ b/service/private/task.js @@ -100,13 +100,21 @@ class __private_task extends Entity { /** * Broadcast a task event to every socket connected to the current hub - * (sender excluded). Silently no-ops if hub_id is missing. + * (the originating socket excluded; every socket of the caller when no + * socket_id was sent). Silently no-ops if hub_id is missing. */ async _broadcast(service, data) { const hub_id = this.hub && this.hub.get(Attr.id); if (!hub_id) return; let dest = await this.yp.await_proc('entity_sockets', hub_id); - dest = toArray(dest).filter((e) => e.uid != this.uid); + // Skip the socket that made this call — it already has the answer — but + // keep the caller's OTHER sessions, so a second tab sees its own user's + // change live. A client that sends no socket_id keeps the old behaviour + // (every socket of the caller skipped), as in chat.react. + const socket_id = this.input.get(Attr.socket_id); + dest = toArray(dest).filter((e) => + socket_id ? e.socket_id != socket_id : e.uid != this.uid, + ); if (isEmpty(dest)) return; await RedisStore.sendData(this.payload(data, { service }), dest); } From e321ce629979360e87b0f02ec5c6419526db6673 Mon Sep 17 00:00:00 2001 From: EddyOne81 Date: Tue, 22 Sep 2026 19:41:34 -0700 Subject: [PATCH 09/34] fix(over-limit): hub.invitations is a read, not a mutation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Access panel's Pending Invitations section came back empty for an org over its seat limit — 401 OVER_LIMIT_READ_ONLY:hub.invitations, measured on the dev endpoint. The clamp classifies a service as mutating by `permission.src > READ_LEVEL`, and hub.invitations is src:'admin' because only an admin may see other people's addresses. It writes nothing. That is precisely the trap the admin./adminpanel. family exemption a few lines below is documented against: src:'admin' is a PRIVILEGE requirement, not a mutation marker. Left as it was, the section would be blank for exactly the org that most needs to read it — one that is over its seat limit and trying to work out who it has outstanding invitations to. hub.invite itself stays clamped, so nothing added here can grow the overage. Co-Authored-By: Claude Opus 5 (1M context) --- router/rest/index.js | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/router/rest/index.js b/router/rest/index.js index 6bae9c0..441bb58 100644 --- a/router/rest/index.js +++ b/router/rest/index.js @@ -90,6 +90,19 @@ const OVER_LIMIT_MUTATING_ALLOWLIST = new Set([ // exception: seeing a migration that was already running when the lock // landed, and STOPPING it, make the overage smaller, not bigger. "google_drive.get_state", "google_drive.cancel", + // 🚨 A PURE READ THAT LOOKS LIKE A MUTATION. hub.invitations lists the + // invitations a workspace is waiting on — it writes nothing — but its + // src is 'admin' because only an admin may see other people's addresses, + // and `mightMutate` is `permission.src > READ_LEVEL`. That is the same trap + // the admin./adminpanel. family exemption below is documented against: + // src:'admin' is a PRIVILEGE requirement, not a mutation marker. + // + // Without this the Access panel's Pending Invitations section is empty for + // exactly the org that most needs to read it — one that is over its seat + // limit and is trying to work out who it has outstanding invitations to. + // hub.invite itself stays clamped, so nothing here can grow the overage. + // Measured on the dev endpoint: 401 OVER_LIMIT_READ_ONLY:hub.invitations. + "hub.invitations", ]); // Once hard-locked, NON-admin members are denied entirely — not even read // (Owner/Admin keep view + resolution access). The FE still needs enough to From 1cdfe6f449bbe0d09cb54526f06af0fe577520d8 Mon Sep 17 00:00:00 2001 From: EddyOne81 Date: Tue, 22 Sep 2026 22:07:57 -0700 Subject: [PATCH 10/34] feat(invite): supersede older invitations when a new one is sent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit _addInviteToken now calls token_hub_invite_supersede after minting, so a person has exactly one live invitation per workspace. The REPLACE inside token_hub_invite_add only covers a re-send by the SAME admin — its unique key carries inviter_id — so a second admin inviting the same address left the first row beside the new one, refusals included. A declined row is parked with expiry 0 so it never ages out, and hub_invitations reports the newest row that still qualifies: once the NEW invitation lapses, the old refusal is the only one left and the panel says "Declined" for an invitation the person never answered. Best-effort: the invitation is already minted and must not fail because a tidy-up did. The worst case is the stale row this removes. Co-Authored-By: Claude Opus 5 (1M context) --- service/private/hub.js | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/service/private/hub.js b/service/private/hub.js index fd6d8ce..4c88ddf 100644 --- a/service/private/hub.js +++ b/service/private/hub.js @@ -1695,6 +1695,26 @@ class __private_hub extends Hub { await this.yp.await_proc( "token_hub_invite_add", email, "", secret, method, this.uid, metadata, expiryTs ); + // ONE LIVE INVITATION PER PERSON PER WORKSPACE. The REPLACE above only + // covers a re-send by the SAME admin — its unique key carries inviter_id — + // so a second admin inviting the same address leaves the first row beside + // the new one, refusals included. token_hub_invite_decline parks a refusal + // with expiry 0 so it never ages out, and hub_invitations reports the + // newest row that still qualifies: once the NEW invitation lapses, the old + // undying refusal is the only one left and the panel says "Declined" for an + // invitation the person never answered. Superseding here is also what was + // asked for — re-inviting somebody who declined turns their row back to + // Pending instead of stacking a second one. + // + // Best-effort: the invitation itself is already minted and must not fail + // because a tidy-up did. The worst case is the stale row this removes. + try { + await this.yp.await_proc( + "token_hub_invite_supersede", email, method, secret + ); + } catch (err) { + this.warn("[hub] invite: superseding older tokens failed", err && err.message); + } return secret; } From d93d4fa53e4a71ce8d350c4af818eb425bd57240 Mon Sep 17 00:00:00 2001 From: EddyOne81 Date: Wed, 23 Sep 2026 03:42:49 -0700 Subject: [PATCH 11/34] feat(invite): stamp invite_status on invitation rows in the feed Answering an invitation only dismisses its notification (the row stays in the history on purpose) and the row keeps its token, so the client kept drawing Accept/Decline after a Decline: pressing Decline again did nothing and Accept reported an invalid link. get_feed now resolves each invitation row's token through token_get_next (the read accept_invite uses) into pending | accepted | declined | expired | invalid. One lookup per distinct token, capped at 20 per page, add-only and best-effort: a failed lookup leaves the field absent and the client falls back to the old token-only behaviour. Pages without invitations make no extra query. Covered by offline/test/hub-invite-status.test.js (32 cases incl. negative controls). Co-Authored-By: Claude Opus 5.5 (1M context) --- acl/activity.json | 4 + offline/test/hub-invite-status.test.js | 278 +++++++++++++++++++++++++ service/lib/hub-invite-status.js | 57 +++++ service/private/activity.js | 58 ++++++ 4 files changed, 397 insertions(+) create mode 100644 offline/test/hub-invite-status.test.js create mode 100644 service/lib/hub-invite-status.js diff --git a/acl/activity.json b/acl/activity.json index 31a6150..279fa18 100644 --- a/acl/activity.json +++ b/acl/activity.json @@ -234,6 +234,10 @@ "lastname": { "type": "string" }, "fullname": { "type": "string" }, "hub_name": { "type": "string" }, + "invite_status": { + "type": "string", + "description": "Workspace invitations only: pending | accepted | declined | expired | invalid. Absent when it could not be resolved; the client then falls back to invite_token alone" + }, "link_label": { "type": "string" }, "task_title": { "type": "string" }, "meeting_action": { "type": "string" }, diff --git a/offline/test/hub-invite-status.test.js b/offline/test/hub-invite-status.test.js new file mode 100644 index 0000000..22bcb06 --- /dev/null +++ b/offline/test/hub-invite-status.test.js @@ -0,0 +1,278 @@ +#!/usr/bin/env node +// +// hub-invite-status.test.js +// +// After a workspace invitation is answered, its notification must stop offering +// Accept and Decline. It did not (Duy, 2026-09-23): the answer only dismisses +// the row, the row keeps its token, and the client drew the buttons for any row +// with a token — so after Decline the row came back unchanged, Decline did +// nothing and Accept reported an invalid link. +// +// The feed now stamps `invite_status` from the token. This exercises: +// 1. the REAL hubInviteStatus (service/lib/hub-invite-status.js), and +// 2. the REAL _stampInviteStatus, sliced out of service/private/activity.js +// and run against a stub yp. +// +// It ends with negative controls: each mutates the sliced SOURCE STRING in +// memory (never the file) to reintroduce a specific mistake and asserts that a +// named case goes red. A test that cannot fail proves nothing. +// +// node offline/test/hub-invite-status.test.js +// +// Exit code 0 = all pass, 1 = any failure. + +const fs = require('fs'); +const path = require('path'); + +const SERVICE_FILE = path.join(__dirname, '..', '..', 'service', 'private', 'activity.js'); +const src = fs.readFileSync(SERVICE_FILE, 'utf8'); +const { hubInviteStatus } = require('../../service/lib/hub-invite-status'); +const MD5_BEFORE = require('crypto').createHash('md5').update(src).digest('hex'); + +let passed = 0; +let failed = 0; +const failures = []; + +function check(label, got, want) { + const ok = JSON.stringify(got) === JSON.stringify(want); + if (ok) { passed++; return true; } + failed++; + failures.push(`${label}\n got ${JSON.stringify(got)}\n want ${JSON.stringify(want)}`); + return false; +} + +const HUB = 'aaaa1111aaaa1112'; +const OTHER_HUB = 'cccc3333cccc3334'; +const NOW = 1_800_000_000; + +function token(over = {}) { + return Object.assign({ + secret: 'sec-1', + method: `hub_invite:${HUB}`, + status: 'active', + expiry: NOW + 3600, + }, over); +} + +// --------------------------------------------------------------------------- +// 1. hubInviteStatus +// --------------------------------------------------------------------------- +check('S1 active, not expired -> pending', hubInviteStatus(token(), HUB, NOW), 'pending'); +check('S2 active, expiry 0 (never) -> pending', hubInviteStatus(token({ expiry: 0 }), HUB, NOW), 'pending'); +check('S3 active, expiry null -> pending', hubInviteStatus(token({ expiry: null }), HUB, NOW), 'pending'); +check('S4 active, past expiry -> expired', hubInviteStatus(token({ expiry: NOW - 1 }), HUB, NOW), 'expired'); +check('S5 active, exactly at expiry -> pending (accept_invite uses now > expiry)', + hubInviteStatus(token({ expiry: NOW }), HUB, NOW), 'pending'); +check('S6 declined -> declined', hubInviteStatus(token({ status: 'declined', expiry: 0 }), HUB, NOW), 'declined'); +check('S7 accepted -> accepted', hubInviteStatus(token({ status: 'accepted' }), HUB, NOW), 'accepted'); +check('S8 accepted, long expired -> still accepted', hubInviteStatus(token({ status: 'accepted', expiry: NOW - 999 }), HUB, NOW), 'accepted'); +check('S9 no token row -> invalid', hubInviteStatus(null, HUB, NOW), 'invalid'); +check('S10 empty row -> invalid', hubInviteStatus({}, HUB, NOW), 'invalid'); +check('S11 not a hub_invite token -> invalid', hubInviteStatus(token({ method: 'signup' }), HUB, NOW), 'invalid'); +check('S12 token for ANOTHER workspace -> invalid', hubInviteStatus(token(), OTHER_HUB, NOW), 'invalid'); +check('S13 row without hub_id is not rejected on that alone', hubInviteStatus(token(), undefined, NOW), 'pending'); +check('S14 unknown status -> invalid', hubInviteStatus(token({ status: 'revoked' }), HUB, NOW), 'invalid'); +check('S15 expiry as a string (driver shape) still compares', + hubInviteStatus(token({ expiry: String(NOW - 5) }), HUB, NOW), 'expired'); + +// --------------------------------------------------------------------------- +// 2. _stampInviteStatus, sliced from the service +// --------------------------------------------------------------------------- +function sliceMethod(marker, what) { + const start = src.indexOf(marker); + if (start < 0) { + console.error(`FATAL: could not find ${what} in ${SERVICE_FILE}`); + process.exit(1); + } + let depth = 0; + let end = -1; + for (let i = src.indexOf('{', start); i < src.length; i++) { + if (src[i] === '{') depth++; + else if (src[i] === '}') { depth--; if (depth === 0) { end = i + 1; break; } } + } + if (end < 0) { console.error(`FATAL: unbalanced braces slicing ${what}`); process.exit(1); } + return src.slice(start, end); +} + +const STAMP_SRC = sliceMethod('\n async _stampInviteStatus(', '_stampInviteStatus'); +for (const needed of ['token_get_next', 'hubInviteStatus', 'MAX_LOOKUPS', 'invite_status']) { + if (!STAMP_SRC.includes(needed)) { + console.error(`FATAL: sliced _stampInviteStatus is missing ${needed} — slicer is wrong`); + process.exit(1); + } +} +// And it must actually be called by get_feed, right after _stampHubInvites. +if (!/await this\._stampHubInvites\(result\);\s*\n\s*await this\._stampInviteStatus\(result\);/.test(src)) { + console.error('FATAL: get_feed does not call _stampInviteStatus right after _stampHubInvites'); + process.exit(1); +} + +const toArray = (a) => { + if (a == null) return []; + if (Array.isArray(a)) return a; + if (typeof a === 'object' && Object.keys(a).length === 0) return []; + return [a]; +}; + +// `tokens` maps secret -> row | Error (throws) | undefined-marker (call failed) +const FAILED = Symbol('failed'); +function harness(tokens = {}, stampSrc = STAMP_SRC) { + const calls = []; + const impl = (new Function('hubInviteStatus', 'toArray', `return { ${stampSrc} };`))( + (row, hub, now) => hubInviteStatus(row, hub, now), toArray, + ); + return { + calls, + debug() { }, + yp: { + async await_proc(name, secret) { + calls.push([name, secret]); + const v = tokens[secret]; + if (v instanceof Error) throw v; + if (v === FAILED) return undefined; + if (v === undefined) return {}; // no such token: empty result + return v; // single row comes back as an object + }, + }, + _stampInviteStatus: impl._stampInviteStatus, + }; +} + +function inviteRow(secret, over = {}) { + return Object.assign({ category: 'hub_invite', hub_id: HUB, invite_token: secret }, over); +} + +(async () => { + const realNow = Date.now; + Date.now = () => NOW * 1000; + try { + // T1 the Duy case: a declined invitation stops being answerable + { + const h = harness({ d: token({ secret: 'd', status: 'declined', expiry: 0 }) }); + const rows = [inviteRow('d')]; + await h._stampInviteStatus(rows); + check('T1 declined row -> declined', rows[0].invite_status, 'declined'); + } + // T2 each status reaches the row + { + const h = harness({ + p: token({ secret: 'p' }), + a: token({ secret: 'a', status: 'accepted' }), + e: token({ secret: 'e', expiry: NOW - 10 }), + }); + const rows = [inviteRow('p'), inviteRow('a'), inviteRow('e'), inviteRow('gone')]; + await h._stampInviteStatus(rows); + check('T2 statuses', rows.map((r) => r.invite_status), ['pending', 'accepted', 'expired', 'invalid']); + } + // T3 one lookup per DISTINCT token, both rows stamped (Unread-ON rollup + raw row) + { + const h = harness({ d: token({ secret: 'd', status: 'declined' }) }); + const rows = [inviteRow('d'), inviteRow('d')]; + await h._stampInviteStatus(rows); + check('T3 one call for a repeated token', h.calls.length, 1); + check('T3 both rows stamped', rows.map((r) => r.invite_status), ['declined', 'declined']); + } + // T4 rows that are not answerable invitations are untouched and cost nothing + { + const h = harness(); + const receipt = { category: 'hub_invite', hub_id: HUB }; // add_contributors receipt + const file = { category: 'mfs', invite_token: 'x' }; // wrong category + const before = JSON.stringify([receipt, file]); + await h._stampInviteStatus([receipt, file, null]); + check('T4 no lookups', h.calls.length, 0); + check('T4 rows byte-identical', JSON.stringify([receipt, file]), before); + } + // T5 a call that FAILED leaves the row without a status (client falls back) + { + const h = harness({ f: FAILED, t: new Error('boom') }); + const rows = [inviteRow('f'), inviteRow('t')]; + await h._stampInviteStatus(rows); + check('T5 failed lookup -> no field', rows.map((r) => 'invite_status' in r), [false, false]); + } + // T6 add-only: an existing status is never overwritten + { + const h = harness({ d: token({ secret: 'd', status: 'declined' }) }); + const rows = [inviteRow('d', { invite_status: 'accepted' })]; + await h._stampInviteStatus(rows); + check('T6 existing field kept', rows[0].invite_status, 'accepted'); + check('T6 no lookup', h.calls.length, 0); + } + // T7 the cap: beyond MAX_LOOKUPS distinct tokens, rows are left unstamped + { + const tokens = {}; + const rows = []; + for (let i = 0; i < 25; i++) { + tokens[`s${i}`] = token({ secret: `s${i}` }); + rows.push(inviteRow(`s${i}`)); + } + const h = harness(tokens); + await h._stampInviteStatus(rows); + check('T7 capped at 20 calls', h.calls.length, 20); + check('T7 first 20 stamped, rest untouched', + rows.map((r) => 'invite_status' in r), + Array.from({ length: 25 }, (_, i) => i < 20)); + } + // T8 token of a different workspace than the row names -> invalid + { + const h = harness({ x: token({ secret: 'x', method: `hub_invite:${OTHER_HUB}` }) }); + const rows = [inviteRow('x')]; + await h._stampInviteStatus(rows); + check('T8 mismatched workspace -> invalid', rows[0].invite_status, 'invalid'); + } + // T9 empty / non-array input is a no-op + { + const h = harness(); + await h._stampInviteStatus([]); + await h._stampInviteStatus(null); + check('T9 no calls', h.calls.length, 0); + } + + // ----------------------------------------------------------------------- + // Negative controls + // ----------------------------------------------------------------------- + async function mustFail(label, from, to, run) { + if (!STAMP_SRC.includes(from)) { + failed++; failures.push(`NC ${label}: mutation anchor not found`); return; + } + const before = failed; + const savedFailures = failures.length; + await run(STAMP_SRC.replace(from, to)); + const wentRed = failed > before; + failed = before; failures.length = savedFailures; + if (wentRed) passed++; + else { failed++; failures.push(`NC ${label}: mutation did NOT turn the case red`); } + } + + await mustFail('treating a failed call as "no token"', 'if (res === undefined) continue;', '', async (s) => { + const h = harness({ f: FAILED }, s); + const rows = [inviteRow('f')]; + await h._stampInviteStatus(rows); + check('NC1', 'invite_status' in rows[0], false); + }); + await mustFail('overwriting an existing status', 'if (r.invite_status != null) continue;', '', async (s) => { + const h = harness({ d: token({ secret: 'd', status: 'declined' }) }, s); + const rows = [inviteRow('d', { invite_status: 'accepted' })]; + await h._stampInviteStatus(rows); + check('NC2', rows[0].invite_status, 'accepted'); + }); + await mustFail('dropping the lookup cap', 'byToken.size < MAX_LOOKUPS', 'true', async (s) => { + const tokens = {}; const rows = []; + for (let i = 0; i < 25; i++) { tokens[`s${i}`] = token({ secret: `s${i}` }); rows.push(inviteRow(`s${i}`)); } + const h = harness(tokens, s); + await h._stampInviteStatus(rows); + check('NC3', h.calls.length, 20); + }); + } finally { + Date.now = realNow; + } + + const MD5_AFTER = require('crypto').createHash('md5') + .update(fs.readFileSync(SERVICE_FILE, 'utf8')).digest('hex'); + check('service file untouched by the test', MD5_AFTER, MD5_BEFORE); + + console.log(`\nhub-invite-status: ${passed} passed, ${failed} failed`); + if (failures.length) { + console.log('\n' + failures.map((f) => '✗ ' + f).join('\n')); + process.exit(1); + } +})(); diff --git a/service/lib/hub-invite-status.js b/service/lib/hub-invite-status.js new file mode 100644 index 0000000..8be5f68 --- /dev/null +++ b/service/lib/hub-invite-status.js @@ -0,0 +1,57 @@ +/** + * @license + * Copyright 2024 Thidima SA. All Rights Reserved. + * Licensed under the GNU AFFERO GENERAL PUBLIC LICENSE, Version 3 + */ + +/** + * Where a workspace invitation stands, read off its token. + * + * WHY THE FEED NEEDS THIS. Answering an invitation stamps `dismissed_at` on its + * notification (contact_activity_dismiss_hub_invite), and dismissal is only + * "acknowledged": the row stays in the activity history on purpose. But the row + * still carries the invitation's secret, and the client draws Accept/Decline + * for any row that has one — so after a Decline the row came back from the + * refresh exactly as it was, Decline pressed again did nothing, and Accept + * reported an invalid link. `dismissed_at` cannot tell the client "answered" + * either: marking a row read writes the same column. + * + * The token is the one place that knows, and it is what both answers write. + * + * pending active and not expired — the only state that is answerable + * accepted the invitation was taken up (including by somebody who already + * held the access it offered — accept_invite redeems it anyway) + * declined turned down + * expired active, but past its expiry: accept_invite would say `expired` + * invalid no such token any more (swept on expiry, or superseded by a + * newer invitation to the same person), or not a hub_invite token + * for this workspace + * + * The expiry rule is accept_invite's own (`expiry > 0 && now > expiry`), so the + * row can never offer an answer the service would then refuse. + * + * @param {object|null} tokenRow a row of yp.token_get_next, or nothing + * @param {string} hub_id the workspace the notification names + * @param {number} now unix seconds + * @returns {'pending'|'accepted'|'declined'|'expired'|'invalid'} + */ +function hubInviteStatus(tokenRow, hub_id, now) { + if (!tokenRow || !tokenRow.secret) return 'invalid'; + const method = String(tokenRow.method || ''); + if (!method.startsWith('hub_invite:')) return 'invalid'; + if (hub_id && method.slice('hub_invite:'.length) !== String(hub_id)) return 'invalid'; + switch (tokenRow.status) { + case 'accepted': + return 'accepted'; + case 'declined': + return 'declined'; + case 'active': { + const expiry = Number(tokenRow.expiry) || 0; + return expiry > 0 && now > expiry ? 'expired' : 'pending'; + } + default: + return 'invalid'; + } +} + +module.exports = { hubInviteStatus }; diff --git a/service/private/activity.js b/service/private/activity.js index 2897d9c..e49e8f7 100644 --- a/service/private/activity.js +++ b/service/private/activity.js @@ -5,6 +5,7 @@ const { Entity } = require('@drumee/server-core'); const { RedisStore, Attr, toArray } = require('@drumee/server-essentials'); const { createHash } = require('node:crypto'); const { resolveHubInviteName } = require('../lib/hub-invite-name'); +const { hubInviteStatus } = require('../lib/hub-invite-status'); const CONTACT_ACTIVITY_CATEGORIES = new Set([ 'contact_refused', 'hub_invite', @@ -1166,6 +1167,7 @@ class MfsActivity extends Entity { // so the rollup rows merged above (which already carry all three) pass // through untouched and the two toggle states agree. await this._stampHubInvites(result); + await this._stampInviteStatus(result); await this._stampChatMentions(result); result = await this._stampMeetingRollups(result); @@ -1594,6 +1596,62 @@ class MfsActivity extends Entity { } } + /** + * Tell the client whether each workspace invitation on the page can still be + * answered, as `invite_status` (see service/lib/hub-invite-status.js). + * + * Without it the row's Accept/Decline outlive the answer: the notification is + * only dismissed, never removed, and it keeps its token — so a declined + * invitation came back from the refresh with both buttons live. The client + * now draws the buttons only for `pending` and a status label otherwise. + * + * Runs AFTER _stampHubInvites, which is what gives the raw Unread-OFF row its + * `category` and `invite_token`; the Unread-ON rollup rows already carry both. + * + * ADD-ONLY and best-effort, like the other stampers. A row whose lookup was + * skipped (over the cap) or failed is left WITHOUT the field, and the client + * treats a missing status exactly as before this existed — buttons whenever + * there is a token — so a failure here can only fall back to the old + * behaviour, never hide an answerable invitation. A lookup that SUCCEEDS but + * finds no token is different: that is a real answer (`invalid`). + * + * One read per distinct token through token_get_next — the same read-only + * proc accept_invite uses — so the verdict matches what pressing the button + * would get. + */ + async _stampInviteStatus(rows) { + const MAX_LOOKUPS = 20; + if (!Array.isArray(rows) || !rows.length) return; + + const byToken = new Map(); // secret -> [rows] + for (const r of rows) { + if (!r || r.category !== 'hub_invite' || !r.invite_token) continue; + if (r.invite_status != null) continue; + const list = byToken.get(r.invite_token); + if (list) list.push(r); + else if (byToken.size < MAX_LOOKUPS) byToken.set(r.invite_token, [r]); + } + if (!byToken.size) return; + + const now = Math.floor(Date.now() / 1000); + for (const [secret, list] of byToken) { + let tokenRow; + try { + const res = await this.yp.await_proc('token_get_next', secret); + // await_proc answers undefined when the call itself failed: that is + // "unknown", not "no such token", so leave these rows alone. + if (res === undefined) continue; + tokenRow = toArray(res)[0] || null; + } catch (e) { + this.debug('[ACTIVITY] invite status lookup failed', e && e.message); + continue; + } + for (const r of list) { + r.invite_status = hubInviteStatus(tokenRow, r.hub_id, now); + } + } + } + /** * Turn a scheduled meeting's rollup row into a MEETING row (Duy 2026-08-21, * issues 9 + 10). From 4ea07dd52e69f140592173a35fd64b40b6eba2aa Mon Sep 17 00:00:00 2001 From: EddyOne81 Date: Wed, 23 Sep 2026 04:33:07 -0700 Subject: [PATCH 12/34] feat(invite): push hub.invitations_changed when an invitation is answered A decline changes no membership, so no push reached the workspace and an admin's open Access panel kept showing the invitation as Pending until it was reopened. _closeInvitation (shared by accept_invite and decline_invite) now ends by pushing hub.invitations_changed to the hub's online members. The payload is the hub id alone: the answer is read back through the admin-gated hub.invitations, so no email rides on a push every member's socket receives. Never throws, like the other member pushes. Co-Authored-By: Claude Opus 5.5 (1M context) --- service/lib/notify-member-joined.js | 38 ++++++++++++++++++++++++++++- service/private/hub.js | 4 ++- 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/service/lib/notify-member-joined.js b/service/lib/notify-member-joined.js index 8f56884..3ba1e75 100644 --- a/service/lib/notify-member-joined.js +++ b/service/lib/notify-member-joined.js @@ -96,4 +96,40 @@ async function notifyMembersChanged(svc, hub_id, { change, users } = {}) { } } -module.exports = { notifyMemberJoined, notifyMembersChanged }; +/** + * Tell every online member of a hub that one of its INVITATIONS was answered + * (accepted or declined), so an open Access panel re-reads its Pending + * Invitations section. + * + * A decline changes no membership, so neither push above fires for it — the + * admin's panel kept showing "Pending" until it was reopened. An accept that + * grants also sends hub.member_joined; this one is still sent, because an + * accept by somebody who already held the access grants nothing and would + * otherwise leave the pending line behind too. + * + * The payload is the workspace id alone: who answered, and how, is read back + * through hub.invitations, which is admin-gated — every member's socket gets + * this push, and it must not carry anybody's email. + * + * Never throws: every call site has already recorded the answer. + * + * @param {object} svc service instance (needs .yp, .payload, .warn) + * @param {string} hub_id hub whose invitation was answered + */ +async function notifyInvitationsChanged(svc, hub_id) { + if (!svc || !hub_id) return; + try { + const dest = toArray(await svc.yp.await_proc("entity_sockets", hub_id)); + if (isEmpty(dest)) return; + await RedisStore.sendData( + svc.payload({ hub_id }, { service: "hub.invitations_changed" }), + dest + ); + } catch (e) { + if (svc.warn) { + svc.warn("[notifyInvitationsChanged] failed for hub", hub_id, e && e.message); + } + } +} + +module.exports = { notifyMemberJoined, notifyMembersChanged, notifyInvitationsChanged }; diff --git a/service/private/hub.js b/service/private/hub.js index e83d2df..f5c96fa 100644 --- a/service/private/hub.js +++ b/service/private/hub.js @@ -28,7 +28,7 @@ const { ID_NOT_FOUND, } = Constants; const { resolve } = require("path"); -const { notifyMemberJoined, notifyMembersChanged } = require("../lib/notify-member-joined"); +const { notifyMemberJoined, notifyMembersChanged, notifyInvitationsChanged } = require("../lib/notify-member-joined"); const { butlerFrom } = require("../lib/mail-sender"); const { mailFailure } = require("../lib/mail-result"); const { resolveHubInviteName } = require("../lib/hub-invite-name"); @@ -2083,6 +2083,8 @@ class __private_hub extends Hub { this.warn("[hub] invitation cleanup: tracking", err && err.message); } } + // LAST, so the panels it wakes read the rows written above. Never throws. + await notifyInvitationsChanged(this, hub_id); } /** From c484f8d7320764cf7a266bd1a26df26007a55af1 Mon Sep 17 00:00:00 2001 From: Aaron Vu Date: Wed, 23 Sep 2026 20:22:00 +0700 Subject: [PATCH 13/34] fix(channel): wait for the sbox copy before purging a staged attachment channel.post copied a staged workspace attachment into the sbox with the detached mfs-copy-node.sh script and, in the same request, purged the staging node with rm -rf. The purge won the race almost every time, so cp found nothing and the committed message pointed at an empty sbox folder: no vignette on the card, and Open hit a 404 on file/orig. Copy the node storage with an awaited fs.cp (following the staging symlink the desk copy leaves while it is still running) so the purge only runs once the bytes are in place. --- service/private/_node-storage.js | 42 ++++++++++++++++++++++++++++++++ service/private/channel.js | 7 +++--- 2 files changed, 46 insertions(+), 3 deletions(-) create mode 100644 service/private/_node-storage.js diff --git a/service/private/_node-storage.js b/service/private/_node-storage.js new file mode 100644 index 0000000..2a21360 --- /dev/null +++ b/service/private/_node-storage.js @@ -0,0 +1,42 @@ +/** + * @license + * Copyright 2024 Thidima SA. All Rights Reserved. + * Licensed under the GNU AFFERO GENERAL PUBLIC LICENSE, Version 3 + */ + +const { MfsTools } = require("@drumee/server-core"); +const { cp } = require("fs/promises"); + +const { check_base, get_base, check_safety } = MfsTools; + +/** + * Copy a node's storage folder and resolve only once the bytes are on disk. + * + * `MfsTools.copy_node(src, dest, 1)` hands the copy to a detached + * `mfs-copy-node.sh` and returns at once. A caller that then deletes the + * source in the same request — channel.post purges the staging copy right + * after copying it into the sbox — races that script: the in-process rm wins, + * `cp -rf src/*` finds nothing, and the sbox node ends up as an empty folder + * that the committed message already references (no vignette, `file/orig` + * 404). Awaiting the copy here makes the later purge safe. + * + * `dereference` matters: while the desk-side staging copy is still running, + * the staging path is a symlink to the original that the script drops at the + * end. Copying the link itself would leave the sbox pointing at a path the + * purge deletes moments later; following it copies the real files. + * + * @param {{nid: string, mfs_root: string}} src + * @param {{nid: string, mfs_root: string}} dest + * @returns {Promise} true when the folder was copied + */ +async function copyNodeStorage(src, dest) { + const from = check_base(src); + if (!from) return false; + const to = get_base(dest); + if (!to) return false; + check_safety(to); + await cp(from, to, { recursive: true, dereference: true }); + return true; +} + +module.exports = { copyNodeStorage }; diff --git a/service/private/channel.js b/service/private/channel.js index 3a72d19..4fc378a 100644 --- a/service/private/channel.js +++ b/service/private/channel.js @@ -19,7 +19,8 @@ const { Attr, RedisStore, toArray, Constants, sysEnv, Script } = require("@drumee/server-essentials"); const { Entity, MfsTools } = require("@drumee/server-core"); -const { remove_node, move_node, copy_node } = MfsTools; +const { remove_node, move_node } = MfsTools; +const { copyNodeStorage } = require("./_node-storage"); const { stampAuthorIdentity } = require("../lib/message-author"); const { movePlanRows } = require("./_move-plan"); const { memberCan, CAN_CHAT } = require("../lib/member-capability"); @@ -355,7 +356,7 @@ class __private_channel extends Entity { tempattachment.push(entry); } if (copy_only) { - await copy_node(src, dest, 1); + await copyNodeStorage(src, dest); } else { await move_node(src, dest); } @@ -374,7 +375,7 @@ class __private_channel extends Entity { } tempattachment.push(entry); } - await copy_node(src, dest, 1); + await copyNodeStorage(src, dest); } } From 35daf43c3c79386dbb10f3b76a119c1ad266ec7b Mon Sep 17 00:00:00 2001 From: Aaron Vu Date: Wed, 23 Sep 2026 20:58:32 +0700 Subject: [PATCH 14/34] docs(chat): record the chat attachment investigations of 22-23 September Debug report covering the From-device attachment leaking into the workspace Files list, the From-workspace picker rework, and the channel.post copy-then-purge race that left sbox attachment folders empty (fixed in c484f8d). --- ...260922-1033-chat-attachment-folder-leak.md | 233 ++++++++++++++++++ 1 file changed, 233 insertions(+) create mode 100644 plans/reports/debug-260922-1033-chat-attachment-folder-leak.md diff --git a/plans/reports/debug-260922-1033-chat-attachment-folder-leak.md b/plans/reports/debug-260922-1033-chat-attachment-folder-leak.md new file mode 100644 index 0000000..dc50c38 --- /dev/null +++ b/plans/reports/debug-260922-1033-chat-attachment-folder-leak.md @@ -0,0 +1,233 @@ +# Debug report: chat attachment lands in workspace Files list; bubble card latency + +Date: 2026-09-22 · Stage: `aaron` endpoint (ui-team `test` @ 6e8a0362, server-team `test` @ 7ffc5c8) + +## Symptom (reported) + +1. Workspace chat (internal + external): attach → From device → send. Message OK, but the + file also shows in the workspace's Files list. Deleting it from Files does not break the + file in the message. Expected: chat attachments never appear in the workspace Files list. +2. After send, the bubble shows the attachment card slowly, sometimes never until the chat + is reloaded by switching workspace. + +## Issue 1 — root cause: CONFIRMED (deliberate behaviour, now unwanted) + +Chain, verified by code + live measurement: + +- `ui-team/src/drumee/builtins/widget/chat/index.js` + - `_getUploadDestination()` (~663): every attachment uploads into the hidden staging + `/__chat__/__upload__/` (`home.chat_upload_id`). Correct. + - `canPromoteDeviceAttachmentsToFolder()` (~703): returns true when `postNid` is set and the + viewer has the write bit. `initialize` (~149) sets `postNid = nid` for **workspace scope + too** (`isHubScopedChat(scope)`), while `scopedNid` stays empty. + - `sendMessage` (~2497): when `getPostNid()` is set, `api.nid = postNid` and + `api.folder_attachment = getPromotableDeviceAttachmentIds(list)` (device uploads only). +- `server-team/service/private/channel.js` `post()` (~1500–1560): + `_classify_staged_attachment` splits staged nodes into `device` (listed in + `folder_attachment`) vs `workspace`. Device nodes are **moved into the folder `nid`** + (`_promote_staged_to_folder` → `mfs_move_all`), then every attachment is **copied** into the + sbox `/__chat__//` (`move_attachemnt` with `copy_only`), staging leftovers of + workspace copies are purged. Result: two nodes — one in Files (original), one in the + message (sbox copy). Deleting the Files one leaves the message copy intact → matches report. + +Commits that introduced it: + +| repo | commit | date | subject | +|---|---|---|---| +| server-team | 61de76e | 2026-06-05 | fix(channel): promote staged folder attachments on post | +| ui-team | 8551402b | 2026-06-05 | fix(chat): keep folder-chat attachments out of the folder until send | +| ui-team | **b04bbf66** | **2026-09-07** | fix(chat): promote workspace-chat uploads into the folder the post writes to (shipped in #560 → #564 PROD 2026-09-09) | + +b04bbf66's stated goal: a device upload with no folder placement had no thread in the folder's +thread rail and "Show in folder" showed nothing. That trade-off is what the user now rejects. + +Live evidence (temptest1, hub "External Workspace" 5688bc445688bc48, db 5_5688bd145688bd15): + +| post variant | uploaded node after post | message node | +|---|---|---| +| `folder_attachment=[nid]` (what the UI sends) | moved to `/probe-promoted.png` (root Files) | `/__chat__//probe-promoted.png` copy | +| no `folder_attachment` | **purged from staging** (not in media, not in trash) | `/__chat__//probe-unpromoted.png` copy | + +Browser flow (attach → From device → send in "Internal Workspace(1)", db 7_cafccfcacafccfcb) +produced the same pair: `/browser-probe.png` at root + sbox copy; card shows "Show in folder". + +Conclusion: the server already implements the wanted behaviour for the no-`folder_attachment` +case. The fix is client-side: stop sending `folder_attachment` (and stop setting `postNid` for +promotion) — at minimum for workspace scope; the DMZ folder-scope (8551402b) needs the same +decision. Side effects to accept: no folder thread-rail entry / "Show in folder" for device +uploads (the b04bbf66 rationale), `tests/chat-upload-promotion.test.js` must be updated. +Server-side `_promote_staged_to_folder` can stay (dead when the client never asks). + +## Issue 2 — root cause CONFIRMED (personal desk folder chat), fix in schemas #186 + +Update 2026-09-22 13:xx: Aaron's failing sends were on his **personal desk** (hub_id == uid, +folder `/Photos`), not on a team hub. Server log for that account: `channel.post` GRANTED → +TERMINATED but **no `chat.attachment` re-fetch afterwards**. API repro on temptest1's own hub: +response carried `_db_err` = "Column 'entity_id' cannot be null" and **no `message_id`**, while +the channel row WAS inserted. Cause: drumate variant of `channel_post_message`, non-hub branch, +bumps `time_channel` with `_entity_id` (P2P peer, only `chat.post` sends it); `channel.post` has +no peer → NULL into NOT NULL PK → EXIT HANDLER returns error JSON instead of the row → client +optimistic bubble never gets `message_id` → no card, until reload loads the stored row. +Explains both "gửi không được" and "bubble hiện nhưng không open được, refresh thì được". +Pre-existing (Aaron's 04:11 rows show the same shape), independent of today's ui change. + +Fix: `drumate/procedures/channel/channel_post_message.sql` — skip the `time_channel` insert when +`_entity_id IS NULL`. Applied to temptest1's DB only (`c_2fb5e0422fb5e043`); after: full row, +`message_id`, `attachment=[{hub_id:,nid}]`, `chat.attachment` 1 item privilege 63. +Pre-fix source: `/root/chat-perm-backup/channel_post_message-drumate-before-fix-20260922.sql`. +PR: https://github.com/drumee/schemas/pull/186 (base preview). Rolled out on Aaron's go +2026-09-22 13:23 to all **450/450** stage drumate DBs that carry the routine (joined through +`yp.entity type='drumate'`), 0 failures, verified by body scan. Bulk `mysql.proc` dump captured +0 rows (the `--where` subquery matched nothing) — rollback reference is the pre-fix source at +`/root/chat-perm-backup/channel_post_message-drumate-before-fix-20260922.sql`, which matched the +body read from temptest1's DB before the change. Post-deploy probe: full row + 1 attachment item. + +Issue 1 fix: ui-team https://github.com/drumee/ui-team/pull/618 (base test), on stage `aaron`. + +### Earlier team-hub measurement (kept for reference) + +Client already has the re-fetch fix (#583, 2026-09-11, `chat-item/index.js _onDataChanged`: +list `restart()` when `message_id` arrives on the optimistic row). Deployed build includes it. + +Measured on stage (probe hooks on XHR/fetch + MutationObserver, temptest1): + +| step | 704 KB PNG (browser) | 12 MB PNG (API) | +|---|---|---| +| media.upload | 3.13 s | 5.83 s | +| channel.post | 0.42 s | 1.12 s | +| chat.attachment (re-fetch) | 0.88–0.94 s | 1.06 s | +| send click → card in bubble | **1.58 s** | n/a (server part is size-independent) | + +Timeline of one send: first `chat.attachment` fires at +159 ms with `message_id` undefined +(expected, evaluated once at list construction), echo arrives +440 ms, `restart()` re-fetch ++625 ms → card at +1.58 s. Server times do not grow with file size (`mfs-copy-node.sh` `cp -rf` +on 12 MB is still ~1.1 s total). + +Unverified hypothesis for the "never shows" case (PLAUSIBLE, not confirmed): `handleReceivedMsg` +matches the echo with `this.echoId == data.echoId`, and `this.echoId` is overwritten by the next +`sendMessage`. A second send before the first response makes the first response miss the +echo branch → appended as a new row while the optimistic row keeps no `message_id` → its card +never loads. Needs a two-quick-sends repro to confirm. Browser tab was reset by the tool when +loading an 11 MB file, so the large-file browser timing is missing. + +## Cleanup + +All probe nodes trashed via `media.trash` (3 in External Workspace, 1 in Internal Workspace(1)). +Probe messages remain in those two temptest chats. Browser session closed. + +## Unresolved questions + +- Should DMZ folder-scoped chat (8551402b) also stop promoting, or only workspace scope? +- Accept losing folder thread-rail / "Show in folder" for device uploads (b04bbf66 rationale)? +- Does the "never shows" case involve sending a second message before the first returns? + +## Issue 3 — "From workspace" picker cannot reach files (reported 14:46) — root cause CONFIRMED, fixed + +Symptom: attach → From workspace lists the rows, clicking a workspace pops "This file type is +not supported"; the flow never reaches a file. + +Cause (by design of the original feature, not a regression): `_openDeskPicker` (ui-team +`widget/chat/index.js`, shipped in `3c88ae0b` 2026-05-17) fed ONE flat `media.show_node_by` +listing of the user's own home root and `_pickDeskFile` rejected every `hub`/`folder` row with +`Wm.alert(FILE_TYPE_NOT_SUPPORTED)`. The rows the user reads as "workspaces" (Photos, +Documents, …) are home-root folders; hub workspaces were in the same list. No drill-down +existed, so only a loose file at the home root was ever pickable. The console lines +`__window_manager: AAA:471 The method *undefined*` are the alert's Close click bubbling to +the window manager — a side effect of the alert, gone with it. + +Fix (ui-team PR `fix/chat-workspace-picker-navigation` → test): the picker now opens on the +desk-sidebar rows (`desk.home type=all`, hub rows gated on area share|private|restricted|public +like `desk_workspace-list`), a hub/folder row pushes onto `_deskPickerTrail` and re-feeds the +list with `media.show_node_by {hub_id, nid}` (hub rows use `actual_home_id`, as +`Wm.loadWorkspace`), a Back control pops the trail, a file row runs the unchanged +copy-to-staging attach path. Rows are typed by class (`--hub`, `--folder`, `--file`). + +Verified on stage (temptest1, chat in External Workspace 5688bc445688bc48): +root → 6 rows (1 personal folder, 3 hubs, 2 probe folders); hub row → Back + contents; folder +row → its file; file row → chip staged; send → bubble card 188 KB. API after send: original +still in the personal folder, External Workspace root unchanged (no Files-list leak). +Seeded probe files trashed afterwards. + +Not covered: the `file/orig//?keysel=regsid` 404s in the user's console — the two `` +ids are not `yp.entity` rows; needs the message ids from the user's account to trace. + +Follow-ups (same day): rows restyled like the @-mention rows (desk icon by area, file +extension, loading skeleton; PR #620 merged), then on Aaron's decision the picker was +re-anchored at the chat's OWN workspace only — team hub → hub home root, personal-desk chat → +its personal workspace folder (`getPostNid`), DMZ share → shared folder; hub rows dropped, +no Back above the root (commit on `test`, verified on stage with temptest1 for the personal +folder chat and the External Workspace chat). + +## Issue 4 — From workspace: card without preview, Open → "A network error has occurred" (2026-09-23) + +Report: upload an image to the workspace (renders and opens fine), attach it via From +workspace, send — the bubble shows the file name but no thumbnail, Open fails with the +generic network error. Aaron: 100% with a fresh upload; intermittent otherwise. + +Evidence (stage, uid 97b24b3d97b24b42, requests land on the `main-service` cluster 93–96): + +| UTC | request | source → staged → sbox node | sbox folder on disk | +|----------|---------------------------------|----------------------------------------------------|---------------------| +| 09:41:34 | media.copy hub b975 (jpg, May) | cc13319f → f6fe1df7 → f7fd5cb2 | **empty** | +| 09:41:52 | media.copy hub 9768 (jpg, May) | a406fb26 → 01c6b6b0 → 02c6a2d3 | **empty** | +| 09:42:25 | media.copy personal (heic, 22/9)| 8992153c → 15798fa6 → cac5/16542d10 | orig + vignette | +| 09:42:40 | media.copy personal (svg, 22/9) | bb413f81 → 1e8fd8b2 → cac5/1fca379e | orig | +| 09:43:08 | media.copy personal (1.png, fresh) | 2a055e82 → 2ee1d66f → cac5/2ff40dba | **empty** | +| 09:49:44 | media.copy personal (2.png, fresh) | 13fe4048 → 1b2509cf → cac5/1eaea68a | orig + vignette (+preview/slide on open) | +| 09:50:31 | media.copy personal (mov, fresh)| 323cdd00 → 3715a9d6 → cac5/38ee451b | only `info.json` written on Open | + +Every failing message row (`1_97b24c1397b24c14.channel.attachment`) points at a sbox node whose +storage folder is EMPTY: no `vignette.png` → card has no preview; no `orig.*` → Open fetches +`file/orig//` → 404 → `LOCALE.ERROR_NETWORK`. The mov additionally raised +`Video FAILED TO RUN SERVICE video.master … null 'join'` (INVALID VIDEO INFO). Yesterday's +console 404 `file/orig/f8fe9787f8fe9789/cac552d4cac552db` is the same thing: message +f8fa969b (2026-09-22 05:53) → sbox node f8fe9787 whose folder has been empty since then. +Fresh upload is NOT the discriminator — two May-2026 workspace files failed the same way and +one fresh png succeeded. It is a race. + +Root cause (server-team `service/private/channel.js`, both `post` and `file_thread_post`): + +1. `move_attachemnt()` copies the staged node into the sbox with `copy_node(src, dest, 1)` + — `detach=1` makes server-core spawn `/usr/share/drumee/bin/mfs-copy-node.sh` detached and + return immediately (`ln -s src dest; mkdir dest.tpm; cp -rf src/* dest.tpm; rm -f dest; + mv dest.tpm dest`). Nothing awaits the bytes. +2. A few ms later, still inside the same request (the whole post took 55 ms: + 09:43:10.073 → .128), `_purge_staged_copies(staged.workspace)` deletes the staging node: + `mfs_attachment_remove` + synchronous `remove_node()` → `rm -rf `. +3. The in-process rm almost always beats bash start-up + `cp`, so `cp -rf src/*` finds no + files, and `mv dest.tpm dest` leaves an empty sbox folder that the committed message + already references. + +Replayed on the stage box with the exact spawn order (script then rm) on scratch dirs: +20/20 destinations empty. Introduced by `111b831` (2026-05-19, detached copy to staging) + +`61de76e` (2026-06-05, purge staged copies after post); deployed on main (server 2.9.94). +The picker's first hop (`media.copy` → `after_transact` → `copy_node(…, 1)`) is the same +detached script but its source is never deleted, so it is not the failing step; direct chat +(`service/private/chat.js`) uses synchronous `copy_node`/`remove_node` and is unaffected. + +Fix direction (not applied): make the sbox copy synchronous in `move_attachemnt` +(`copy_node(src, dest)` → `cpSync`, same as chat.js) or, if the detached script must stay, +purge the staging node only after the copy has landed (await the child / verify `orig.*` +exists in the sbox folder before `_purge_staged_copies`). Existing broken cards +(2ff40dba, 38ee451b, f8fe9787 in hub cac552d4) keep empty folders; their staging sources are +already purged, so they cannot be repaired from disk. + +Fix applied (server-team `c484f8d` on `test`, 2026-09-23): new `service/private/_node-storage.js` +exports `copyNodeStorage(src, dest)` — an awaited `fs/promises.cp` (recursive, `dereference` +so a staging path that is still the desk copy's symlink is followed) built on MfsTools +`check_base`/`get_base`/`check_safety`; `channel.js move_attachemnt` uses it in place of both +`copy_node(src, dest, 1)` calls, so `_purge_staged_copies` only runs once the sbox bytes are +on disk. Desk copy (`private/media.js`), direct chat (`chat.js`) and the shell script are +untouched. + +Verified on stage `aaron` (temptest1, personal folder c6157887c615788c, API replay of the UI +flow: fresh upload → 1.5 s → `media.copy` → 1.5 s → `channel.post`): 3/3 sbox folders hold +`orig.png` + `vignette.png`, `file/orig` and `file/vignette` answer 200 for all three, the +three staging copies are purged, no orphan folders. Awaited-copy replay of the spawn order on +the stage box: 0/20 empty (was 20/20). Test uploads and staging leftovers trashed; the six +probe messages in temptest1's personal folder chat could not be deleted — +`channel.delete` fails there with `PROCEDURE c_2fb5e0422fb5e043.channel_delete_hub_all does +not exist` (stage schema gap, pre-existing, not touched). + +Still open: the `main` endpoint Aaron tests on runs the old code until `test` is deployed +there; cards already broken (`2ff40dba`, `38ee451b`, `f8fe9787` in hub cac552d4) stay empty. From dfeec3b3969d08a2e80fa74dec88f11742264e3d Mon Sep 17 00:00:00 2001 From: "phamtobao@gmail.com" Date: Wed, 23 Sep 2026 23:28:28 +0400 Subject: [PATCH 15/34] feat(upload): chunked, parallel, resumable uploads (upload_init/chunk/status/complete/abort) --- acl/media.json | 356 ++++++++++++++++++++++++++++++++++ service/lib/chunked-upload.js | 195 +++++++++++++++++++ service/media.js | 182 +++++++++++++++++ 3 files changed, 733 insertions(+) create mode 100644 service/lib/chunked-upload.js diff --git a/acl/media.json b/acl/media.json index 4eb5d9e..9aa7a86 100644 --- a/acl/media.json +++ b/acl/media.json @@ -120,6 +120,362 @@ } ] }, + "upload_init": { + "doc": "Chunked upload step 1 - open a resumable session for a large file; quota and folder permission are checked here before any content is sent", + "scope": "hub", + "permission": { + "src": "write", + "preproc": { + "checker": "pre_upload" + } + }, + "params": { + "nid": { + "type": "string", + "required": true, + "doc": "Destination folder ID (0 = hub home)" + }, + "filename": { + "type": "string", + "required": true, + "maxLength": 126, + "doc": "Original filename" + }, + "filesize": { + "type": "number", + "required": true, + "doc": "Total file size in bytes" + }, + "replace": { + "type": "number", + "required": false, + "enum": [ + 0, + 1 + ], + "default": 0, + "doc": "Replace existing file (1) or create new (0)" + }, + "ownpath": { + "type": "string", + "required": false, + "doc": "Absolute path within hub (parent folders are created if missing)" + } + }, + "returns": { + "upload_id": { + "type": "string", + "doc": "Session id to send with every following call" + }, + "chunk_size": { + "type": "number", + "doc": "Bytes per chunk (16 MB)" + }, + "total": { + "type": "number", + "doc": "Number of chunks the client must send" + }, + "received": { + "type": "array", + "doc": "Always empty for a new session" + } + }, + "errors": [ + { + "code": "INVALID_FILESIZE", + "message": "filesize is missing or not a positive integer", + "http_status": 400 + }, + { + "code": "INSUFFICIENT_STORAGE", + "message": "Not enough storage quota", + "http_status": 507 + }, + { + "code": "PERMISSION_DENIED", + "message": "No write permission on destination folder", + "http_status": 403 + }, + { + "code": "OWNPATH_INCONSISTENT", + "message": "ownpath must use home_id as base", + "http_status": 400 + } + ] + }, + "upload_chunk": { + "doc": "Chunked upload step 2 - store one chunk of an open session at its offset; the body is streamed like media.upload (request carries upload=1) and chunks may arrive in any order or in parallel", + "scope": "hub", + "permission": { + "src": "write", + "preproc": { + "checker": "pre_upload" + } + }, + "params": { + "nid": { + "type": "string", + "required": true, + "doc": "Destination folder ID (0 = hub home)" + }, + "upload_id": { + "type": "string", + "required": true, + "doc": "Session id returned by upload_init" + }, + "index": { + "type": "number", + "required": true, + "doc": "Zero-based chunk index" + }, + "file": { + "type": "file", + "required": true, + "doc": "Chunk content (application/octet-stream), exactly chunk_size bytes except for the last chunk" + } + }, + "returns": { + "upload_id": { + "type": "string", + "doc": "Session id" + }, + "index": { + "type": "number", + "doc": "Index just stored" + }, + "received": { + "type": "array", + "doc": "Indices of the chunks already stored on the server" + } + }, + "errors": [ + { + "code": "UPLOAD_SESSION_NOT_FOUND", + "message": "No open session with this upload_id (expired after 48h or aborted)", + "http_status": 404 + }, + { + "code": "PERMISSION_DENIED", + "message": "Session belongs to another user or hub", + "http_status": 403 + }, + { + "code": "BAD_CHUNK_INDEX", + "message": "index is outside 0..total-1", + "http_status": 400 + }, + { + "code": "BAD_CHUNK_SIZE", + "message": "Chunk body length does not match the expected length for this index", + "http_status": 400 + } + ] + }, + "upload_status": { + "doc": "Chunked upload - which chunks of a session have already landed; the client calls it after a reload to resume with only the missing chunks", + "scope": "hub", + "permission": { + "src": "write", + "preproc": { + "checker": "pre_upload" + } + }, + "params": { + "nid": { + "type": "string", + "required": true, + "doc": "Destination folder ID (0 = hub home)" + }, + "upload_id": { + "type": "string", + "required": true, + "doc": "Session id returned by upload_init" + } + }, + "returns": { + "upload_id": { + "type": "string", + "doc": "Session id" + }, + "filename": { + "type": "string", + "doc": "Filename given at upload_init" + }, + "filesize": { + "type": "number", + "doc": "Total size given at upload_init" + }, + "chunk_size": { + "type": "number", + "doc": "Bytes per chunk" + }, + "total": { + "type": "number", + "doc": "Number of chunks" + }, + "received": { + "type": "array", + "doc": "Indices of the chunks already stored on the server" + } + }, + "errors": [ + { + "code": "UPLOAD_SESSION_NOT_FOUND", + "message": "No open session with this upload_id (expired after 48h or aborted)", + "http_status": 404 + }, + { + "code": "PERMISSION_DENIED", + "message": "Session belongs to another user or hub", + "http_status": 403 + } + ] + }, + "upload_complete": { + "doc": "Chunked upload step 3 - commit the assembled file into the MFS through the same store path as media.upload (quota, changelog, live update, indexing); when chunks are still missing the reply carries error INCOMPLETE and their indices instead", + "scope": "hub", + "permission": { + "src": "write", + "preproc": { + "checker": "pre_upload" + } + }, + "params": { + "nid": { + "type": "string", + "required": true, + "doc": "Destination folder ID (0 = hub home)" + }, + "upload_id": { + "type": "string", + "required": true, + "doc": "Session id returned by upload_init" + }, + "filename": { + "type": "string", + "required": true, + "maxLength": 126, + "doc": "Original filename" + }, + "filesize": { + "type": "number", + "required": true, + "doc": "Total file size in bytes (quota check)" + }, + "replace": { + "type": "number", + "required": false, + "enum": [ + 0, + 1 + ], + "default": 0, + "doc": "Replace existing file (1) or create new (0)" + }, + "ownpath": { + "type": "string", + "required": false, + "doc": "Absolute path within hub" + } + }, + "returns": { + "id": { + "type": "string", + "doc": "New file node ID" + }, + "nid": { + "type": "string", + "doc": "New file node ID (alias)" + }, + "filename": { + "type": "string", + "doc": "Stored filename" + }, + "filesize": { + "type": "number", + "doc": "File size in bytes" + }, + "error": { + "type": "string", + "doc": "INCOMPLETE when chunks are missing (then no file is created)" + }, + "missing": { + "type": "array", + "doc": "Indices still to send, only with error INCOMPLETE" + } + }, + "errors": [ + { + "code": "UPLOAD_SESSION_NOT_FOUND", + "message": "No open session with this upload_id (expired after 48h or aborted)", + "http_status": 404 + }, + { + "code": "PERMISSION_DENIED", + "message": "Session belongs to another user or hub", + "http_status": 403 + }, + { + "code": "UPLOAD_CORRUPTED", + "message": "Assembled file size does not match filesize; the session was dropped, start over", + "http_status": 500 + }, + { + "code": "INSUFFICIENT_STORAGE", + "message": "Not enough storage quota", + "http_status": 507 + }, + { + "code": "FAILED_CREATE_FILE", + "message": "File creation failed", + "http_status": 500 + } + ] + }, + "upload_abort": { + "doc": "Chunked upload - cancel a session and delete its partial file", + "scope": "hub", + "permission": { + "src": "write", + "preproc": { + "checker": "pre_upload" + } + }, + "params": { + "nid": { + "type": "string", + "required": true, + "doc": "Destination folder ID (0 = hub home)" + }, + "upload_id": { + "type": "string", + "required": true, + "doc": "Session id returned by upload_init" + } + }, + "returns": { + "upload_id": { + "type": "string", + "doc": "Session id" + }, + "aborted": { + "type": "number", + "doc": "Always 1" + } + }, + "errors": [ + { + "code": "UPLOAD_SESSION_NOT_FOUND", + "message": "No open session with this upload_id (expired after 48h or aborted)", + "http_status": 404 + }, + { + "code": "PERMISSION_DENIED", + "message": "Session belongs to another user or hub", + "http_status": 403 + } + ] + }, "download": { "doc": "Download file from MFS", "scope": "hub", diff --git a/service/lib/chunked-upload.js b/service/lib/chunked-upload.js new file mode 100644 index 0000000..6417eb1 --- /dev/null +++ b/service/lib/chunked-upload.js @@ -0,0 +1,195 @@ +/** + * Chunked, resumable upload sessions (media.upload_init / upload_chunk / + * upload_status / upload_complete / upload_abort). + * + * A plain media.upload rides one request for the whole file: on a slow link a + * multi-GB file is a multi-hour request that restarts from zero on any network + * blip. Here the browser cuts the file into CHUNK_SIZE pieces and sends them + * as independent small requests (in parallel, each with its own retries). The + * server keeps the session in Redis and the bytes in ONE preallocated sparse + * file under /chunked/, writing every chunk at its own offset, so + * chunks can land in any order and a session survives a page reload: the + * client asks upload_status which chunks are already there and sends only the + * missing ones. upload_complete hands the assembled file to the very same + * store() a normal upload ends in, so quota, changelog, live update and + * indexing behave exactly like a single-request upload. + * + * Redis layout (node-redis v4+ API): + * upload:session: hash {upload_id, uid, hub_id, nid, filename, filesize, + * chunk_size, total, path, replace, ownpath, ctime} + * upload:chunks: set indices already written + * Both keys carry SESSION_TTL, refreshed on every chunk. + */ +const { join, extname } = require("node:path"); +const { mkdirSync, createReadStream, createWriteStream } = require("node:fs"); +const { open, stat, unlink, readdir } = require("node:fs/promises"); +const { RedisStore, sysEnv } = require("@drumee/server-essentials"); + +const { tmp_dir } = sysEnv(); + +const CHUNK_SIZE = 16 * 1024 * 1024; +const SESSION_TTL = 48 * 3600; // seconds +const DIRNAME = "chunked"; +const SESSION_KEY = "upload:session:"; +const CHUNKS_KEY = "upload:chunks:"; +const SWEEP_EVERY_MS = 3600 * 1000; + +let lastSweep = 0; + +function client() { + const c = RedisStore.getClient(); + if (!c) throw new Error("REDIS_UNAVAILABLE"); + return c; +} + +function sessionDir() { + const dir = join(tmp_dir, DIRNAME); + mkdirSync(dir, { recursive: true }); + return dir; +} + +function normalize(s) { + return { + ...s, + filesize: Number(s.filesize), + chunk_size: Number(s.chunk_size), + total: Number(s.total), + replace: Number(s.replace || 0), + }; +} + +/** + * Open a session and preallocate the target as a sparse file (no disk used + * until chunks are written). + */ +async function createSession({ upload_id, uid, hub_id, nid, filename, filesize, replace, ownpath }) { + const total = Math.max(1, Math.ceil(filesize / CHUNK_SIZE)); + const path = join(sessionDir(), `${upload_id}${extname(filename || "")}`); + const fh = await open(path, "w"); + try { + await fh.truncate(filesize); + } finally { + await fh.close(); + } + const sess = { + upload_id, + uid: String(uid), + hub_id: String(hub_id || ""), + nid: String(nid || ""), + filename: String(filename), + filesize: String(filesize), + chunk_size: String(CHUNK_SIZE), + total: String(total), + path, + replace: String(replace ? 1 : 0), + ownpath: String(ownpath || ""), + ctime: String(Date.now()), + }; + const c = client(); + await c.hSet(SESSION_KEY + upload_id, sess); + await c.expire(SESSION_KEY + upload_id, SESSION_TTL); + sweep().catch(() => { }); + return normalize(sess); +} + +async function getSession(upload_id) { + if (!upload_id) return null; + const s = await client().hGetAll(SESSION_KEY + upload_id); + if (!s?.upload_id) return null; + return normalize(s); +} + +async function received(upload_id) { + const m = await client().sMembers(CHUNKS_KEY + upload_id); + return (m || []).map(Number).filter(Number.isInteger).sort((a, b) => a - b); +} + +async function missing(sess) { + const got = new Set(await received(sess.upload_id)); + const out = []; + for (let i = 0; i < sess.total; i++) if (!got.has(i)) out.push(i); + return out; +} + +function expectedLength(sess, index) { + return Math.min(sess.chunk_size, sess.filesize - index * sess.chunk_size); +} + +/** + * Copy an incoming chunk file into the session file at its offset. Parallel + * chunks write disjoint ranges through separate descriptors, so no lock. + */ +function writeChunk(sess, index, src) { + const start = index * sess.chunk_size; + return new Promise((resolve, reject) => { + const r = createReadStream(src); + const w = createWriteStream(sess.path, { flags: "r+", start }); + r.on("error", reject); + w.on("error", reject); + w.on("finish", resolve); + r.pipe(w); + }); +} + +async function markReceived(upload_id, index) { + const c = client(); + await c.sAdd(CHUNKS_KEY + upload_id, String(index)); + await c.expire(CHUNKS_KEY + upload_id, SESSION_TTL); + await c.expire(SESSION_KEY + upload_id, SESSION_TTL); +} + +/** + * Drop the Redis keys; remove the session file unless store() already moved it. + */ +async function destroySession(sess, keepFile = false) { + if (!sess?.upload_id) return; + try { + await client().del([SESSION_KEY + sess.upload_id, CHUNKS_KEY + sess.upload_id]); + } catch (e) { /* redis gone: the TTL cleans up */ } + if (!keepFile) await removeQuietly(sess.path); +} + +async function removeQuietly(path) { + if (!path) return; + try { + await unlink(path); + } catch (e) { /* already gone */ } +} + +/** + * Session files whose Redis keys expired (abandoned uploads) would otherwise + * stay on disk forever: once an hour, drop those older than SESSION_TTL. + */ +async function sweep() { + const now = Date.now(); + if (now - lastSweep < SWEEP_EVERY_MS) return; + lastSweep = now; + const dir = sessionDir(); + let names = []; + try { + names = await readdir(dir); + } catch (e) { + return; + } + for (const name of names) { + const p = join(dir, name); + try { + const st = await stat(p); + if (now - st.mtimeMs > SESSION_TTL * 1000) await unlink(p); + } catch (e) { /* raced with a live upload */ } + } +} + +module.exports = { + CHUNK_SIZE, + SESSION_TTL, + createSession, + getSession, + received, + missing, + expectedLength, + writeChunk, + markReceived, + destroySession, + removeQuietly, +}; diff --git a/service/media.js b/service/media.js index dcf7145..76ad2a7 100644 --- a/service/media.js +++ b/service/media.js @@ -26,6 +26,7 @@ const { memberCan, CAN_DOWNLOAD } = require("./lib/member-capability"); const { notifyHubActivity } = require("./lib/activity-mailer"); const { markFunnelMilestone } = require("./lib/funnel-milestone"); const { markFeatureUsage } = require("./lib/feature-usage"); +const ChunkedUpload = require("./lib/chunked-upload"); const { DENIED } = Events; const { BATCH_FILE, @@ -75,6 +76,7 @@ const { rename: renameAsync, copyFile: copyFileAsync, unlink: unlinkAsync, + stat: statAsync, } = require("fs/promises"); /** @@ -735,6 +737,186 @@ class __media extends Mfs { await this.store(parent.id, filepath, this.input.need(Attr.filename)); } + // --------------------------------------------------------------------------- + // Chunked, resumable upload: the browser cuts files of 64 MB and more into + // 16 MB pieces and sends them as independent requests (parallel, each with + // its own retries), so a network blip costs one chunk instead of the whole + // file and a reload resumes where it stopped. Session mechanics live in + // service/lib/chunked-upload.js; the five services below are the ACL + // surface. upload_init and upload_complete share the `upload` contract + // (write + pre_upload) because they are the two steps that touch the target + // folder; the per-chunk calls only need a session, and a session is bound + // to the uid and hub that opened it. + // --------------------------------------------------------------------------- + + /** + * Destination folder of a chunked upload, resolved the way upload() does it: + * nid "0" is the hub home, and pre_upload (ownpath) may have created the + * real parent and left it in heap.upload. + * @returns {string} + */ + _chunkedTargetNid() { + let nid = this.input.use(Attr.nid); + if (nid == "0") nid = this.home_id; + if (this.heap.upload?.nid) nid = this.heap.upload.nid; + return nid; + } + + /** + * The session named by upload_id, or null after answering the client when + * it does not exist or belongs to another user or hub. + * @returns {Promise} + */ + async _chunkedSession() { + const upload_id = this.input.need("upload_id"); + const sess = await ChunkedUpload.getSession(upload_id); + if (!sess) { + this.exception.user("UPLOAD_SESSION_NOT_FOUND"); + return null; + } + const hub_id = this.hub.get(Attr.id); + if (String(sess.uid) !== String(this.uid) || String(sess.hub_id) !== String(hub_id)) { + this.warn(`chunked upload ${upload_id}: session owner mismatch`); + this.exception.user("PERMISSION_DENIED"); + return null; + } + return sess; + } + + /** + * Chunked upload, step 1: open a session. Quota and folder permission are + * settled here, before a single byte of content travels. + */ + async upload_init() { + let filename = this.input.need(Attr.filename); + try { + filename = decodeURI(filename); + } catch (e) { /* raw name with a stray percent sign: keep it */ } + const filesize = Number.parseInt(this.input.need(Attr.filesize), 10); + if (!Number.isInteger(filesize) || filesize <= 0) { + return this.exception.user("INVALID_FILESIZE"); + } + const folder = this.granted_node(); + if (isEmpty(folder) || !folder.id) { + return this.exception.user("PERMISSION_DENIED"); + } + if (!(await this.chekcDiskLimit())) return; + const sess = await ChunkedUpload.createSession({ + upload_id: this.randomString(), + uid: this.uid, + hub_id: this.hub.get(Attr.id), + nid: this._chunkedTargetNid(), + filename, + filesize, + replace: this.shouldReplace(), + ownpath: this.input.get(Attr.ownpath), + }); + this.output.data({ + upload_id: sess.upload_id, + filename, + filesize, + chunk_size: sess.chunk_size, + total: sess.total, + received: [], + }); + } + + /** + * Chunked upload, step 2 (once per chunk). The body was streamed by core/io + * into a tmp file (the request carries upload:1, like media.upload); it is + * copied into the session file at index * chunk_size, then removed. + */ + async upload_chunk() { + const sess = await this._chunkedSession(); + if (!sess) return; + const index = Number.parseInt(this.input.need("index"), 10); + const incoming = this.input.need(Attr.uploaded_file); + if (!Number.isInteger(index) || index < 0 || index >= sess.total) { + await ChunkedUpload.removeQuietly(incoming); + return this.exception.user("BAD_CHUNK_INDEX"); + } + let size = -1; + try { + size = (await statAsync(incoming)).size; + } catch (e) { /* no tmp file: reported as a size mismatch below */ } + const expected = ChunkedUpload.expectedLength(sess, index); + if (size !== expected) { + this.warn(`chunked upload ${sess.upload_id}: chunk ${index} is ${size} bytes, expected ${expected}`); + await ChunkedUpload.removeQuietly(incoming); + return this.exception.user("BAD_CHUNK_SIZE"); + } + try { + await ChunkedUpload.writeChunk(sess, index, incoming); + } finally { + await ChunkedUpload.removeQuietly(incoming); + } + await ChunkedUpload.markReceived(sess.upload_id, index); + this.output.data({ + upload_id: sess.upload_id, + index, + received: await ChunkedUpload.received(sess.upload_id), + }); + } + + /** + * Which chunks have landed. The client asks after a reload, to send only + * the missing ones. + */ + async upload_status() { + const sess = await this._chunkedSession(); + if (!sess) return; + this.output.data({ + upload_id: sess.upload_id, + filename: sess.filename, + filesize: sess.filesize, + chunk_size: sess.chunk_size, + total: sess.total, + received: await ChunkedUpload.received(sess.upload_id), + }); + } + + /** + * Chunked upload, step 3: commit. With chunks still missing the client gets + * their indices back (error INCOMPLETE) and sends them; otherwise the + * assembled file goes through the same store()/replace() a single-request + * upload ends in, so quota, changelog, live update and indexing are shared. + */ + async upload_complete() { + const sess = await this._chunkedSession(); + if (!sess) return; + const missing = await ChunkedUpload.missing(sess); + if (missing.length) { + return this.output.data({ upload_id: sess.upload_id, error: "INCOMPLETE", missing }); + } + let size = -1; + try { + size = (await statAsync(sess.path)).size; + } catch (e) { /* file vanished from tmp */ } + if (size !== sess.filesize) { + this.warn(`chunked upload ${sess.upload_id}: assembled ${size} bytes, expected ${sess.filesize}`); + await ChunkedUpload.destroySession(sess); + return this.exception.user("UPLOAD_CORRUPTED"); + } + const nid = this._chunkedTargetNid(); + if (this.shouldReplace() && isFunction(this.replace)) { + await this.replace(this.granted_node().id, sess.path, sess.filename); + } else { + await this.store(nid, sess.path, sess.filename); + } + // store()/replace() moved the file into the MFS; only the keys remain. + await ChunkedUpload.destroySession(sess, true); + } + + /** + * Cancelled by the user: drop the session and its half-written file. + */ + async upload_abort() { + const sess = await this._chunkedSession(); + if (!sess) return; + await ChunkedUpload.destroySession(sess); + this.output.data({ upload_id: sess.upload_id, aborted: 1 }); + } + /** * */ From 7d76460ba1e76436120e62416e6db61e90ec5ea9 Mon Sep 17 00:00:00 2001 From: Aaron Vu Date: Thu, 24 Sep 2026 23:12:13 +0700 Subject: [PATCH 16/34] perf(channel): move staged attachments into the sbox and list them all A post used to copy every verified staging node into the sbox with a detached script, then delete the staging node in the same request, and chat.attachment handed the bubble a fixed page of five that nothing ever paged past. A 36-file message took 5.85 s server-side and showed 5 cards. Verified staging nodes are now moved (mfs_move_all + rename) so the cost no longer depends on file size and no purge step remains; nodes the classifier did not vouch for keep the copy path. The classifier looks up all nids in one query, and chat.attachment returns the whole list. Stage replay: 36 files post in 1.2 s, 36 cards, no empty sbox folders, no staging leftovers. --- service/private/channel.js | 127 +++++++++++++++++++++++++++++++++---- service/private/chat.js | 7 +- 2 files changed, 119 insertions(+), 15 deletions(-) diff --git a/service/private/channel.js b/service/private/channel.js index 4fc378a..7c8d1a9 100644 --- a/service/private/channel.js +++ b/service/private/channel.js @@ -312,9 +312,18 @@ class __private_channel extends Entity { message_id, copy_only = false, folderNids = null, + staged = false, ) { let src = []; message_id = [message_id]; + // `staged`: every source is a node the caller verified to sit in the hub's + // chat staging folder (_classify_staged_attachment). Such a node exists + // only to become this message's attachment, so it is MOVED into the sbox + // — mfs_move_all re-parents (same DB) or re-creates and deletes (cross + // DB) the row, and move_node renames the storage folder, O(1) whatever + // the file size — instead of being copied and then purged from staging. + // That copy-then-purge pair is what raced and left empty sbox folders, + // and what made a post cost the full copy time of every attachment. // Sources promoted into the folder: tag their sbox copy with the folder file // nid so reply-in-thread and the folder's "View Chat Threads" resolve to ONE // thread (keyed by the folder file F, not the per-message sbox copy C). @@ -358,6 +367,13 @@ class __private_channel extends Entity { if (copy_only) { await copyNodeStorage(src, dest); } else { + // A hub that has never held a file has no __storage__ yet, and + // mv() needs the parent of the destination to exist. + if (node.des_mfs_root) { + try { + mkdirSync(node.des_mfs_root, { recursive: true }); + } catch (_) {} + } await move_node(src, dest); } break; @@ -393,8 +409,10 @@ class __private_channel extends Entity { } } // In copy_only mode the originals still exist alongside the sbox copies; - // pushing both here would render each attachment twice in the chat. - if (!copy_only && this.hub.get(Attr.id) != this.uid) { + // pushing both here would render each attachment twice in the chat. A + // staged source no longer exists after the move, so it has nothing to + // reference either. + if (!copy_only && !staged && this.hub.get(Attr.id) != this.uid) { for (let media of attachment) { tempattachment.push({ nid: media, hub_id: this.hub.get(Attr.id) }); } @@ -402,6 +420,74 @@ class __private_channel extends Entity { return tempattachment; } + /** + * Put a post's attachments into the message's sbox folder. + * + * Verified staging nodes (`stagedNids`) are moved there — cheap and final. + * Anything else (a file promoted into the scoped folder, or a node the + * classifier did not vouch for) is copied so the original stays where it + * is. Returns the attachment entries in the order the client sent them. + */ + async _attach_to_sbox( + sbox, + desdir, + attachment, + message_id, + copy_only, + promoted, + stagedNids, + ) { + const stagedSet = new Set(toArray(stagedNids).map(String)); + const toMove = []; + const toCopy = []; + for (const nid of toArray(attachment)) { + (stagedSet.has(`${nid}`) ? toMove : toCopy).push(nid); + } + const entries = {}; + if (!isEmpty(toCopy)) { + const rows = await this.move_attachemnt( + sbox, + desdir, + toCopy, + message_id, + copy_only, + promoted, + ); + // move_attachemnt reports destination ids only; a copied node keeps its + // source order in the plan, so pair them positionally. + toCopy.forEach((nid, i) => { + if (rows[i]) entries[`${nid}`] = rows[i]; + }); + // Legacy branch of move_attachemnt may append source references after + // the destinations; keep them, they carry no source nid to pair with. + for (const row of rows.slice(toCopy.length)) { + entries[`${row.hub_id}:${row.nid}`] = row; + } + } + if (!isEmpty(toMove)) { + const rows = await this.move_attachemnt( + sbox, + desdir, + toMove, + message_id, + false, + null, + true, + ); + toMove.forEach((nid, i) => { + if (rows[i]) entries[`${nid}`] = rows[i]; + }); + } + const ordered = []; + for (const nid of toArray(attachment)) { + if (entries[`${nid}`]) ordered.push(entries[`${nid}`]); + } + for (const key of Object.keys(entries)) { + if (key.includes(":")) ordered.push(entries[key]); + } + return ordered; + } + /** * Folder-scoped posts: split staged attachments (/__chat__/__upload__/) * into device uploads (client asked to promote them into the folder via @@ -416,12 +502,22 @@ class __private_channel extends Entity { const staging_id = mfs_home && mfs_home.chat_upload_id; if (!staging_id) return res; const wanted = new Set(toArray(folder_attachment).map(String)); - for (let nid of toArray(attachment)) { - let rows = await this.db.await_query( - "SELECT id, parent_id, owner_id, origin_id, category FROM media WHERE id=?", - `${nid}`, - ); - let node = toArray(rows)[0]; + const nids = toArray(attachment).map(String).filter(Boolean); + if (isEmpty(nids)) return res; + // One round trip for the whole list: a 36-file post used to spend 36 + // queries here. + const rows = await this.db.await_query( + `SELECT id, parent_id, owner_id, origin_id, category FROM media WHERE id IN (${nids + .map(() => "?") + .join(",")})`, + ...nids, + ); + const byId = {}; + for (const row of toArray(rows)) { + if (row && row.id) byId[`${row.id}`] = row; + } + for (let nid of nids) { + let node = byId[nid]; // Anchor on the actual staging parent — not a file_path substring, // which a user-created folder literally named __chat__ could spoof. if (!node || `${node.parent_id}` !== `${staging_id}`) continue; @@ -1568,13 +1664,14 @@ class __private_channel extends Entity { "mfs_make_dir", `'${sbox.chat_id}','${stringify([message_id])}',1`, ); - attachment = await this.move_attachemnt( + attachment = await this._attach_to_sbox( sbox, desdir, attachment, message_id, copy_only, promoted, + staged.workspace, ); } input.author_id = this.uid; @@ -1617,9 +1714,8 @@ class __private_channel extends Entity { data.is_attachment = 1; } // Only after the message and its attachment records are committed: - // remove the now-redundant staging copies and surface the promoted - // files in everyone's open folder window. - await this._purge_staged_copies(staged.workspace); + // surface the promoted files in everyone's open folder window. Staged + // copies were moved into the sbox, so there is nothing left to purge. await this._notify_folder_new_nodes(promoted, nid); if (!isEmpty(thread_id)) { @@ -2039,12 +2135,14 @@ class __private_channel extends Entity { "mfs_make_dir", `'${sbox.chat_id}','${stringify([message_id])}',1`, ); - attachment = await this.move_attachemnt( + attachment = await this._attach_to_sbox( sbox, desdir, attachment, message_id, copy_only, + promoted, + staged.workspace, ); } @@ -2099,7 +2197,8 @@ class __private_channel extends Entity { ); data.is_attachment = 1; } - await this._purge_staged_copies(staged.workspace); + // Staged copies were moved into the sbox; only the promoted files need + // announcing to open folder windows. await this._notify_folder_new_nodes(promoted, folder_nid); // Refresh thread summary + root card metadata (reply_count, last_message, mtime). diff --git a/service/private/chat.js b/service/private/chat.js index 8c9c6d3..b3209af 100644 --- a/service/private/chat.js +++ b/service/private/chat.js @@ -60,6 +60,11 @@ class privateChat extends Entity { async attachment() { let message_id = this.input.use(Attr.message_id); let peer_id = this.input.use(Attr.peer_id); + // The whole list, not a page of five. The chat bubble shows every + // attachment of a message and its card list never scrolls, so the old + // fixed page hid every file past the fifth. `page` is still read and + // echoed on each row because the list widget sends it and the card + // model carries it; it no longer selects a slice. let page = this.input.use(Attr.page) || 1; let attach = {}; let data = await this.db.await_proc("channel_get", message_id); @@ -76,7 +81,7 @@ class privateChat extends Entity { if (!isEmpty(data) && !isEmpty(data.attachment)) { data.attachment = this.parseJSON(data.attachment); - attach = data.attachment.slice((page - 1) * 5, page * 5); + attach = data.attachment; if (!isEmpty(attach)) { attach = await this._getAttachmentsInfo(attach, this.uid, page); } From 53fd1b9a8dd0d6391e598d13d0c3f4c777fdc4ee Mon Sep 17 00:00:00 2001 From: Aaron Vu Date: Thu, 24 Sep 2026 23:12:49 +0700 Subject: [PATCH 17/34] docs(chat): record the multi-file attachment investigation and fix directions --- ...-260924-2225-chat-multi-attachment-perf.md | 82 +++++++++++++++++++ ...260922-1033-chat-attachment-folder-leak.md | 37 +++++++++ 2 files changed, 119 insertions(+) create mode 100644 plans/reports/brainstorm-260924-2225-chat-multi-attachment-perf.md diff --git a/plans/reports/brainstorm-260924-2225-chat-multi-attachment-perf.md b/plans/reports/brainstorm-260924-2225-chat-multi-attachment-perf.md new file mode 100644 index 0000000..9c376f3 --- /dev/null +++ b/plans/reports/brainstorm-260924-2225-chat-multi-attachment-perf.md @@ -0,0 +1,82 @@ +--- +title: Chat multi-file attachments — performance and UX fix directions +date: 2026-09-24 +status: proposed +related: plans/reports/debug-260922-1033-chat-attachment-folder-leak.md (Issue 4, Issue 5) +--- + +# Summary + +Two measured defects on a 36-file post (stage, 2026-09-24): the bubble shows only 5 cards +(`chat.attachment` pages 5 at a time, the bubble list never asks for page 2) and the post +takes 5.85 s server-side (~160 ms/file, copy + purge of every staging node in-request). The +awaited copy shipped in `c484f8d` fixes the empty-folder race but makes heavy posts slower. +Recommendation: return the whole attachment list, MOVE staging nodes into the sbox instead of +copy + purge, batch the per-file DB work, and render the sender's cards optimistically. + +# Contract + +- **Outcome:** every attachment of a message is shown; sending N files (incl. a 800 MB video) + returns in well under a second of server time apart from DB work; the sender sees cards + immediately, not a skeleton for the duration of the post. +- **Constraints:** `chat.attachment` response shape stays an array of node rows (web + chat-item, litechat, mobile `decodeAttachmentInfoRows` consume it); DB access through + stored procedures only; staging nodes must never survive in `__chat__/__upload__`; + attachments never land in the workspace Files list (2026-09-22 decision). +- **Non-goals:** repairing already-broken cards; changing upload itself; touching the desk + copy (`private/media.js`) or the shell script. +- **Acceptance:** 36-file post → 36 cards; `channel.post` server time for 36 small files + < 1 s and independent of file size (rename, not copy); 0 empty sbox folders over 20 posts; + no `__chat__/__upload__` leftovers; mobile `test:live` chat contract still green. + +# Options + +## A. Attachment list: return everything vs. page from the client +1. **Server returns the full list** when `page` is not sent (keep paging only for an explicit + `page`). One-line change in `chat.js attachment()`; `_getAttachmentsInfo` does one + `mfs_access_node` per node — 36 forward_proc calls ≈ 36 × 2–3 ms, fine. Fails first if a + message carries hundreds of files (nobody does; upload UI has no cap either). **Recommended.** +2. Client keeps requesting pages until `_e.eod`. Touches ui-core list behaviour or a custom + loop in chat-item, still 8 round trips for 36 files, and mobile stays at 5. Rejected. + +## B. Post cost: move vs. copy +1. **Move staging nodes into the sbox** (`mfs_move_all` + `move_node` = rename). The plan + procedure already emits `'move'` rows for both same-DB (UPDATE parent_id) and cross-DB + (create in dest + DELETE in source), so no purge step and no DB row left behind; the + copy-then-purge race disappears with it. `_classify_staged_attachment` guarantees the + nodes are staging copies, so the historical reason for `copy_only` (originals posted + directly) no longer applies. Fails first on EXDEV (staging and sbox on different volumes): + `mv()` falls back to `cpSync` + `rmSync`, still correct, just slower. **Recommended.** +2. Keep copy_only but stream it (`fs.cp` awaited — current state). Correct but O(size); + 844 MB in-request. Rejected as the end state. +3. Copy detached, purge later (delayed job). Adds a scheduler and a window where the sbox is + empty. Rejected. + +Also batch the per-file DB round trips inside the post: one `SELECT … WHERE id IN (?)` in +`_classify_staged_attachment` instead of N, and one `mfs_move_all` call for all nodes (already +the case). Expected: post time dominated by `channel_post_attachment`'s loop. + +## C. Sender UX +1. **Optimistic cards from the composer's staged items**: the composer already holds + `nid/hub_id/filetype/ext` (media_grid cards); feed them to the bubble list instead of + firing `chat.attachment` before the post returns (today that call answers empty and the + skeleton pulses for the whole post). Reconcile on the post reply: same-hub moves keep the + nid (URLs stay valid); cross-DB moves return new nids → re-feed once. **Recommended.** +2. Keep the skeleton but suppress the premature fetch and show "Sending N files…" with the + upload-progress window. Cheaper, still a wait. Fallback if C1 proves fragile. + +# Recommendation and order + +1. server `chat.js attachment()`: full list unless `page` given (A1). +2. server `channel.js move_attachemnt` + post/file_thread_post: `mfs_move_all` + `move_node` + for staged nodes, drop `_purge_staged_copies` for them, batch the classify SELECT (B1). + `_node-storage.js` stays only if a copy path remains; otherwise remove it. +3. ui-team chat-item/chat: optimistic cards + reconcile (C1). + +Verify with the API replay used for Issue 4 (upload → copy → post) extended to 36 files and +one ≥ 500 MB file, plus the stage browser run. + +# Unresolved +- Whether any client still relies on `channel.post` promoting device uploads into the folder + (`folder_attachment`); web sends none, mobile type allows it — confirm before removing the + promote branch (not required for this fix, the move keeps it working). diff --git a/plans/reports/debug-260922-1033-chat-attachment-folder-leak.md b/plans/reports/debug-260922-1033-chat-attachment-folder-leak.md index dc50c38..21c9602 100644 --- a/plans/reports/debug-260922-1033-chat-attachment-folder-leak.md +++ b/plans/reports/debug-260922-1033-chat-attachment-folder-leak.md @@ -231,3 +231,40 @@ not exist` (stage schema gap, pre-existing, not touched). Still open: the `main` endpoint Aaron tests on runs the old code until `test` is deployed there; cards already broken (`2ff40dba`, `38ee451b`, `f8fe9787` in hub cac552d4) stay empty. + +## Issue 5 — many files in one message: long wait, only 5 cards (2026-09-24, lexishoang.drumee.in) + +Evidence (stage `main-service`, uid 70ba905970ba905d, share hub a8083fe4a8083feb, folder +a8957928a8957932, 09:52–10:06 UTC): + +| UTC | post GRANTED → TERMINATED | files stored in `channel.attachment` | on disk | +|----------|---------------------------|--------------------------------------|---------| +| 09:52:47 | 62 ms | 1 | ok | +| 09:55:14 | 405 ms | 10 | 10/10 have bytes | +| 10:01:15 | **5 849 ms** | **36** | 36/36 have bytes (one mp4 = 844 MB) | +| 10:01:41 | 17 ms | text: "ủa sao upload 1 đống mà còn ít" | — | + +So the server kept every attachment; nothing was dropped at post time. + +Root cause of "only 5": `server-team/service/private/chat.js attachment()` pages the stored +list five at a time — `attach = data.attachment.slice((page - 1) * 5, page * 5)` — and the +bubble's card list (`ui-team widget/chat-item/index.js`, `Skeletons.List.Smart` with +`api: getAttachments`, `flow: none`, no height) never scrolls, so the base list's +`_onScroll` paging never asks for page 2. Log confirms one `chat.attachment` call per message +(page 1, 2–10 ms) and no page 2. Every message with more than 5 attachments shows 5. + +Root cause of "long wait": the bubble is drawn optimistically and fires `chat.attachment` +at once (10:01:15.988, message not stored yet → empty → skeleton), then again only after +`channel.post` returns (10:01:22.166). The post itself took 5.85 s for 36 files (0.4 s for +10): per-file work inside one request — `_classify_staged_attachment` SELECT per nid, +`mfs_copy_all` over 36 nodes, 36 detached copy spawns, `_purge_staged_copies` (36 × +`mfs_attachment_remove` + `rm -rf`), `channel_post_attachment` loop. Roughly 160 ms per file. +Note for the fix shipped in `c484f8d` (awaited `fs.cp` instead of the detached script): on a +message like this one the 844 MB copy now runs inside the request as well, so heavy posts get +slower still. The staging node is purged right after the copy, so a rename +(`move_node`, O(1) on the same volume) instead of copy + purge would remove both the race +and the copy cost; `mfs_move_all` emits copy+delete rows for cross-DB moves (personal hub → +sbox hub), so that path needs a rename fallback for those rows. + +Fix direction (not applied): (1) `chat.attachment` returns the whole list (or the bubble +requests pages until `_e.eod`); (2) replace copy + purge of staging nodes with a move. From 25e20807e69de7fa3b501268094c557fce770be6 Mon Sep 17 00:00:00 2001 From: Aaron Vu Date: Thu, 24 Sep 2026 23:18:30 +0700 Subject: [PATCH 18/34] fix(channel): keep same-hub staged moves as message attachments mfs_move_all reports a move inside one hub as 'show'/'same' with no 'move' row, so a staged node moved into a team or share hub's own sbox produced no attachment entry. The node keeps its id there, so the sources themselves are the entries. --- service/private/channel.js | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/service/private/channel.js b/service/private/channel.js index 7c8d1a9..427cff4 100644 --- a/service/private/channel.js +++ b/service/private/channel.js @@ -408,10 +408,19 @@ class __private_channel extends Entity { } } } + // A move inside one hub (team or share hub: its sbox is the hub itself) + // only re-parents the rows — mfs_move_all reports it as 'show'/'same' + // with no 'move' row, the node keeps its id and its storage folder — so + // the attachment entries are the sources themselves. + if (staged && `${this.hub.get(Attr.id)}` === `${sbox.hub_id}`) { + for (let media of attachment) { + tempattachment.push({ nid: `${media}`, hub_id: sbox.hub_id }); + } + } // In copy_only mode the originals still exist alongside the sbox copies; // pushing both here would render each attachment twice in the chat. A - // staged source no longer exists after the move, so it has nothing to - // reference either. + // staged source moved to another hub no longer exists, so it has nothing + // to reference either. if (!copy_only && !staged && this.hub.get(Attr.id) != this.uid) { for (let media of attachment) { tempattachment.push({ nid: media, hub_id: this.hub.get(Attr.id) }); From fab1dab560933a1c2e1e1af7a5a61b241455d661 Mon Sep 17 00:00:00 2001 From: Aaron Vu Date: Thu, 24 Sep 2026 23:41:10 +0700 Subject: [PATCH 19/34] docs(chat): record the multi-file attachment fixes and stage results --- ...260922-1033-chat-attachment-folder-leak.md | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/plans/reports/debug-260922-1033-chat-attachment-folder-leak.md b/plans/reports/debug-260922-1033-chat-attachment-folder-leak.md index 21c9602..b33a560 100644 --- a/plans/reports/debug-260922-1033-chat-attachment-folder-leak.md +++ b/plans/reports/debug-260922-1033-chat-attachment-folder-leak.md @@ -268,3 +268,31 @@ sbox hub), so that path needs a rename fallback for those rows. Fix direction (not applied): (1) `chat.attachment` returns the whole list (or the bubble requests pages until `_e.eod`); (2) replace copy + purge of staging nodes with a move. + +Fix applied (2026-09-24, order A1 → B1 → C1 from +`plans/reports/brainstorm-260924-2225-chat-multi-attachment-perf.md`): + +- A1 `7d76460` — `chat.js attachment()` returns the whole stored list; `page` is only echoed on + the rows (the ui-core list always sends `page=1`, so opt-in paging was not an option). +- B1 `7d76460` + `25e2080` — `channel.js`: verified staging nodes are MOVED into the sbox + (`mfs_move_all` + rename) through `_attach_to_sbox`; anything the classifier did not vouch + for keeps the awaited copy; `_purge_staged_copies` no longer runs after a post; the + classifier looks the nids up in one query. Same-hub moves (team/share hub) carry no `'move'` + row, the node keeps its id, so the sources are the entries. +- C1 ui-team (`test`) — composer hands the staged node rows to the optimistic bubble + (`attachment_preview`, `media-wrapper.getAttachmentNodes()` stripped of picker/strip widget + fields); the bubble feeds them to its card list, retires the placeholder at once, and skips + the refetch when the echo's entries match; a cross-hub move refetches with `start(0)` so + the cards never blank. + +Stage results (aaron): 12 files incl. 15 MB → post 1.25 s, 12 cards, 12/12 folders with +bytes, 0 staging rows left; 36 files → 1.2 s (was 5.85 s), 36 cards; team hub 8 files → 8 +cards, 8/8 bytes. Browser (temptest1): team hub send → card at 164–176 ms, placeholder gone, +0 `chat.attachment` calls; personal folder send → card at 178 ms, 1 `chat.attachment`, no +blank while the stored rows replace the preview. Seeded files trashed; probe messages remain +in temptest1's chats. + +Side notes: the picker's copy shows an "Uploading 1 file" progress row that can linger with a +"Cancel" control when the same file is picked twice; the ui `upload` of files through the +hidden `input[type=file]` did not trigger the messenger's handler in automation, so the +device-upload path was covered by the API replay only. From add8aa30cedb957879346ad369471e6739ba5533 Mon Sep 17 00:00:00 2001 From: Aaron Vu Date: Fri, 25 Sep 2026 02:12:10 +0700 Subject: [PATCH 20/34] docs(plan): conference.invite through the mobile push pipeline --- .../phase-01-admit-conference-invite.md | 61 ++++++++++++++ .../phase-02-stage-verification.md | 39 +++++++++ .../plan.md | 80 +++++++++++++++++++ 3 files changed, 180 insertions(+) create mode 100644 plans/260923-1745-conference-mobile-push/phase-01-admit-conference-invite.md create mode 100644 plans/260923-1745-conference-mobile-push/phase-02-stage-verification.md create mode 100644 plans/260923-1745-conference-mobile-push/plan.md diff --git a/plans/260923-1745-conference-mobile-push/phase-01-admit-conference-invite.md b/plans/260923-1745-conference-mobile-push/phase-01-admit-conference-invite.md new file mode 100644 index 0000000..9c57d7a --- /dev/null +++ b/plans/260923-1745-conference-mobile-push/phase-01-admit-conference-invite.md @@ -0,0 +1,61 @@ +--- +phase: 1 +title: "Admit conference.invite + policy + tests + merge vào test" +status: pending +priority: P1 +effort: "1d" +dependencies: [] +--- + +# Phase 1: Admit conference.invite vào mobile-push + +## Overview + +Cho `conference.invite` đi vào pipeline `admit → mobilePushQueue → mobilePushWorker → FCM` +với TTL ngắn và nội dung ring. + +## Requirements + +- `service/lib/mobile-push.js`: + - `ALLOWED_EVENTS` (:7-13) += `'conference.invite'`. + - **`normalizeEvent` (:35-47) dựng object cố định và bỏ key lạ** → thêm `room_id` (validate bằng `ID`, :15) và `room_type` (chuỗi ngắn), nếu không `data.room_id` luôn rỗng. Test: `room_id` sống tới job `deliver` (`mobilePushWorker.js:119,124-131`). +- `service/conference.js` `invite()` (`room_id` là input tuỳ chọn :567 nhưng cả web lẫn mobile luôn gửi): + - Admit `{type:'conference.invite', actor_id: this.uid, hub_id, key_id: room_id, room_id, room_type: (this.input.get(Attr.metadata)||{}).type, recipient_uids:[guest_id], expires_at: Math.floor(Date.now()/1000) + 60}` — **`expires_at` là epoch giây** (`mobile-push.js:44`), không phải ms. + - Ở ba nhánh: `data.offline` (~589), `clients` rỗng (~609), live (~638). **Không** push ở nhánh `cross_call` (:572-583). Chỉ admit khi có `room_id`. + - Fire-and-forget: lỗi admit chỉ log, response `invite` không đổi. + - Ghi nhận: nhánh live cấp quyền rendez-vous 24 h cho guest (`conference_invite.sql:35-54`), nhánh offline không cấp → người được mời từ push chỉ Join được nếu là member hub (mobile chỉ mời member hub → chấp nhận). +- `service/lib/mobile-push-recipients.js`: `conference.invite` dùng shape `hub_id` + `recipient_uids` (không vào `BROADCAST_EVENTS`). Recipient tường minh hiện bị giao với ≤100 member đầu (trang 45, :2-3,29-46,58-63) → với event không broadcast, kiểm từng uid bằng `hubRecipientAllowed` sẵn có thay vì giao phân trang (không SQL mới). +- `service/lib/mobile-push-content.js`: `EVENT_BODY['conference.invite'] = () => 'Is calling you'` (giá trị là **hàm**, :50-57,121 — chuỗi sẽ rơi về `GENERIC_NOTIFICATION`); thêm vào `WORKSPACE_SUBTITLED`; title = `push_actor_name`. +- `service/lib/mobile-push-policy.js` `buildFcmMessage` + `offline/workers/mobilePushWorker.js` `sendFcm`: + - `data` += `room_id`, `actor_id`, `room_type`, **`expires_at`** (epoch giây, để app bỏ ring cũ), tất cả `String(...)`. + - **`sendFcm` kiểm hết hạn trước khi gọi FCM**: `if (Number(delivery.expires_at) <= nowSec) return {expired:true}` — retry backoff 60 s × 4 (`mobilePushQueue.js:33-36`) có thể chạy sau hạn; `ttl` âm → `INVALID_ARGUMENT` → `permanentFcmError` → `device_registration_v2_invalidate` (`mobilePushWorker.js:166-173`) **xoá đăng ký push của máy cho mọi loại push**. + - Android `ttl = max(0, expires_at − nowSec) + 's'` (thay hard-code `'3600s'` :41); event không set `expires_at` giữ mặc định. + - Gộp banner: Android **`android.notification.tag = room_id`** (thay banner đang hiện; `collapse_key` chỉ gộp tin xếp hàng khi offline — tuỳ chọn); iOS header `apns-collapse-id = room_id` (≤64 B). + - Giữ `apns-push-type: 'alert'` (data-only không đánh thức app terminated). +- TTL giới hạn **giao tin**, không thu hồi banner đã hiện; không có push huỷ khi caller cúp/callee trả lời trên web — chấp nhận ở v1 (app chặn tap cũ bằng `expires_at`). +- Không raw SQL, không stored procedure mới. + +## Related Code Files + +- Modify: `service/conference.js`, `service/lib/mobile-push.js`, `service/lib/mobile-push-content.js`, `service/lib/mobile-push-policy.js` +- Modify thêm: `offline/workers/mobilePushWorker.js` (`sendFcm` expiry guard), `service/lib/mobile-push-recipients.js` (explicit recipient check) +- Tests: mở rộng `test/mobile-push-*.test.js`: admission (`room_id`/`room_type` sống qua normalize, `expires_at` giây), policy (ttl từ expires_at, `notification.tag`, `apns-collapse-id`, data string), worker (delivery hết hạn → không gọi FCM, không invalidate), content (title/subtitle/body), recipients (invitee ngoài 100 member đầu vẫn nhận) + +## Implementation Steps + +1. Nhánh `feat/conference-mobile-push` từ `test` mới nhất. +2. Policy + content + whitelist + test (thuần, không cần stage). +3. Admit trong `invite()` ba nhánh; review không đổi response shape `{offline:1}` / payload socket. +4. Chạy các test node; `node -e "require('./service/conference')"` smoke load. +5. Commit conventional không AI trailer; **merge vào `test`** (Aaron 2026-09-24), push `origin/test`. + +## Success Criteria + +- [ ] Test standalone xanh (admission / policy / content). +- [ ] Response `conference.invite` không đổi (diff chỉ thêm admit). +- [ ] Đã merge vào `test` và push; message commit nêu hành vi, không nêu plan ID. + +## Risk Assessment + +- Người bị ring nhiều lần (spam invite): collapse theo `room_id` + TTL 60 s giới hạn; rate-limit ngoài scope. +- `occurred_at` khác nhau giữa các lần → `event_id` khác → không dedupe server: chấp nhận, collapse OS lo phần hiển thị. diff --git a/plans/260923-1745-conference-mobile-push/phase-02-stage-verification.md b/plans/260923-1745-conference-mobile-push/phase-02-stage-verification.md new file mode 100644 index 0000000..772ae74 --- /dev/null +++ b/plans/260923-1745-conference-mobile-push/phase-02-stage-verification.md @@ -0,0 +1,39 @@ +--- +phase: 2 +title: "Deploy stage + verify với app mobile" +status: pending +priority: P1 +effort: "0.5d" +dependencies: [1] +--- + +# Phase 2: Verify trên stage + +## Overview + +Sync `test` lên stage `aaron` (tìm cách deploy ở bước đầu, ghi vào plan Unresolved 1 / memory nếu Aaron xác nhận), verify end-to-end với drumee-mobile (Android +emulator có Play services nhận FCM thật; iOS simulator không nhận APNs thật → +iOS để lượt máy thật ở bảng evidence roadmap). + +## Requirements + +- Chỉ tài khoản `temptest1`–`temptest10@drumee.com`; prelive read-only; credentials không echo/log. +- Kịch bản (B = người gọi, A = app mobile trên emulator): + 1. A background → B `conference.invite` → banner OS ≤ 5 s, title = tên B, subtitle "To {workspace}", body "Is calling you". + 2. A terminated (force-stop) → như trên; tap → cold start → tab Meet đúng workspace (+ ring nếu còn cửa sổ — phase 12 mobile). + 3. A foreground → không banner OS thêm; ring socket vẫn hiện một lần. + 4. B ring hai lần liên tiếp → Android thay banner cũ (`notification.tag`), không thành hai. + 5. A offline 90 s rồi online → không banner muộn (không giao sau `expires_at`). + 6. A vừa background < 1 phút (socket còn sống, Android FGS) → ring socket + banner OS; tap banner → không ring lần hai (dedupe `room_id`). + 7. A trả lời trên web → banner trên phone vẫn còn (chấp nhận ở v1); tap sau 60 s → chỉ mở tab Meet. + 8. Không có registration nào bị invalidate trong log sau các kịch bản trên. +- Log `mobilePushWorker` cho event (không in token/registration id). + +## Success Criteria + +- [ ] 8 kịch bản pass, ghi kết quả + thời gian vào report `plans/reports/`. +- [ ] Không lỗi mới trong log push worker / conference. + +## Risk Assessment + +- Stage chưa có service account FCM hợp lệ cho bản stage app → push không tới dù code đúng; kiểm `chat.post` push trước làm baseline. diff --git a/plans/260923-1745-conference-mobile-push/plan.md b/plans/260923-1745-conference-mobile-push/plan.md new file mode 100644 index 0000000..9c1ff6d --- /dev/null +++ b/plans/260923-1745-conference-mobile-push/plan.md @@ -0,0 +1,80 @@ +--- +title: "Push OS cho cuộc gọi đến (conference.invite) tới app mobile" +description: "Cho conference.invite đi qua pipeline mobile-push (FCM/APNs) để phone có app background/terminated vẫn nhận banner 'X is calling you'; TTL ngắn, collapse theo room; không đổi stored procedure." +status: pending +priority: P1 +effort: "1d + verify" +branch: "feat/conference-mobile-push → merge into test" +tags: [mobile-push, conference, fcm, apns, drumee-mobile] +created: 2026-09-24 +blockedBy: [] +blocks: ["drumee-mobile:260920-0842-drumee-mobile-meeting-jitsi (phase 12)"] +research: "../../../drumee-mobile/plans/260920-0842-drumee-mobile-meeting-jitsi/reports/researcher-260924-conference-push.md" +--- + +# Push OS cho cuộc gọi đến + +## Overview + +Hôm nay `conference.invite` / `conference.start` chỉ đi qua socket live +(`RedisStore.sendData`); `service/conference.js` không gọi `admitMobilePush`. +Khi callee không có socket, stored proc `conference_invite` trả `{offline:1}` +và `invite()` (`conference.js:~589`, `~609`) dừng — không push, không gì cả. +Test Android emulator 2026-09-24 xác nhận: app background → ring mất hẳn. + +Pipeline mobile-push đã gửi `notification` block native +(`mobile-push-policy.js:23-63`) nên chỉ cần **cho event vào**: whitelist, nội +dung, data keys, TTL ngắn, và gọi admit ở `invite()`. Không cần đổi schemas +(mọi lookup `hub_members_for_mobile_push`, `push_registration_*`, +`push_actor_name`, `push_workspace_name` đã có). + +Yêu cầu từ Aaron 2026-09-24 ("lên kế hoạch cho server-team"); app phía mobile +là phase 12 của plan `drumee-mobile/plans/260920-0842-drumee-mobile-meeting-jitsi`. + +## Goals + +| # | Goal | Priority | +|---|------|----------| +| 1 | Mỗi `conference.invite` thành công hoặc `{offline:1}` → admit push `conference.invite` cho `guest_id` | P1 | +| 2 | Push ring **không được giao** sau 60 s (Android `ttl` + APNs expiration, worker bỏ delivery hết hạn — không invalidate registration), gộp banner theo `room_id` (`notification.tag` / `apns-collapse-id`) | P1 | +| 3 | Banner "{Caller} · To {workspace} · Is calling you"; data có `room_id`, `actor_id`, `room_type`, `expires_at` (qua `normalizeEvent`) | P1 | +| 4 | Test standalone cho admission / policy / content | P1 | + +## Non-goals + +- VoIP/PushKit, CallKit, Android full-screen intent (ring khi máy ngủ) — ngoài scope (Aaron 2026-09-24). +- `conference.start` push — **không làm** (Aaron 2026-09-24). +- Opt-out / mute setting cho push (pipeline chưa có cho event nào). +- Sửa vi phạm raw-SQL sẵn có ở `mobile-push-authorization.js:37-49` (ghi nhận, không mở rộng). + +## Phases + +| # | Phase | Status | +|---|-------|--------| +| 1 | [Admit conference.invite + policy + tests + merge vào test](./phase-01-admit-conference-invite.md) | Pending | +| 2 | [Deploy stage + verify với app mobile](./phase-02-stage-verification.md) | Pending | + +## Success Criteria + +- [ ] temptest B `conference.invite` temptest A khi app A background **và** terminated → A thấy banner OS trong ≤ 5 s; tap mở app vào tab Meet đúng workspace. +- [ ] Push không được giao sau 60 s (tắt mạng máy A 90 s rồi bật → không banner); không registration nào bị invalidate. +- [ ] Ring lặp cùng room chỉ còn một banner. +- [ ] Test node standalone xanh; không raw SQL mới; merge vào `test`, stage `aaron` chạy bản mới. + +## Red Team Review + +2026-09-24: phát hiện server trong [`drumee-mobile/.../reports/red-team-260924-follow-ups.md`](../../../drumee-mobile/plans/260920-0842-drumee-mobile-meeting-jitsi/reports/red-team-260924-follow-ups.md) (C1, C2, H1, H2, M4–M7, M11, L6, L7) — tất cả đã sửa vào phase 1–2. + +## Validation Log + +### Session 1 — 2026-09-24 (Aaron) + +| # | Câu hỏi | Quyết định | +|---|---|---| +| 1 | Push `conference.start` | **Không push** — chỉ người được mời đích danh (`conference.invite`); meeting có lịch dùng `room.reminder` sẵn có | +| 2 | Ai làm, nhánh nào | **Làm luôn, merge thẳng vào `test`** (nhánh feature local → merge `test`, không chờ PR), **sync lên stage `aaron`** để test | +| 3 | Tên người gọi trong `data` | Không thêm (title banner đã có tên; app tra từ `actor_id`) — mặc định, không phải PII mới | + +## Unresolved questions + +1. Cách sync code server-team lên stage `aaron` (deploy script / restart service) — tìm ở đầu phase 2, ghi lại cho lần sau. From 8a013684d6f1bfa7026754733f12a388afdcd22b Mon Sep 17 00:00:00 2001 From: EddyOne81 Date: Thu, 24 Sep 2026 20:17:02 -0700 Subject: [PATCH 21/34] feat(room): carry hub_id and pid on the room.scheduled push MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The invitation popup gets a "View Calendar" button that opens the meeting in its workspace calendar. The push only carried the meeting's nid, which is a node id inside ONE hub's database and cannot be opened without the hub. Add hub_id and the parent folder — the same hub_id/pid the durable meeting_notice row already carries. Additive only; no schema change. Co-Authored-By: Claude Opus 5.5 --- service/private/room.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/service/private/room.js b/service/private/room.js index ec69fec..2456bb2 100644 --- a/service/private/room.js +++ b/service/private/room.js @@ -291,6 +291,13 @@ class __private_room extends __public_room { recur, from: name, folder_name, + // The card's "View Calendar" button opens this meeting in its + // workspace calendar. `nid` alone cannot do that — it is a node id + // inside ONE hub's database — so the hub travels with it, and the + // parent folder so the pane lands where the meeting is filed (the same + // hub_id/pid the durable meeting_notice row carries). + hub_id: this.hub.get(Attr.id), + pid: (node && node.parent_id) || null, // The card's meta line counts who is invited ("N invited") and shows // their faces, exactly as the reminder's does. `attendees` is already // the normalised { uid, name } list built above, so this costs From d6fa56f083ae664b9c5e3d6766ed11a7b3565abf Mon Sep 17 00:00:00 2001 From: Tran Hoang Huan <121786621+tranh0anghuan@users.noreply.github.com> Date: Fri, 25 Sep 2026 11:00:17 +0700 Subject: [PATCH 22/34] feat(env): tours_new_user_since cutoff for contextual tours (#232) Expose myDrumee.tours_new_user_since (unix seconds) as a platform flag. The client compares it with the account's entity.ctime so contextual tutorial tours are offered to new accounts only. 0/absent keeps today's behaviour (every account eligible), so deploy order is free. Co-authored-by: Drumee Dev Co-authored-by: Claude Opus 5.5 (1M context) --- service/lib/env.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/service/lib/env.js b/service/lib/env.js index d4ce40d..708ae51 100644 --- a/service/lib/env.js +++ b/service/lib/env.js @@ -153,6 +153,13 @@ function platform() { // single post-signup tour and no trigger fires, so this is a true kill // switch — the client writes nothing while it is 0. platform.contextual_tours = global.myDrumee.contextual_tours ? 1 : 0; + // Unix seconds. Contextual tours are for NEW users only: an account whose + // entity.ctime (get_user → data.user.ctime) is older than this is never + // offered one. The client compares the two, so no backfill of + // tutorials_seen is needed for existing accounts. 0/absent = no cutoff, + // every account is eligible — today's behaviour, which makes the rollout + // order free: ship, then set the date in myDrumee.json. + platform.tours_new_user_since = ~~global.myDrumee.tours_new_user_since; platform.cdnHost = global.myDrumee.cdnHost; platform.version = global.VERSION; platform.TfaMethods = TfaMethods; From b2f48816009cc84766d5db0ac9031ca2200cc203 Mon Sep 17 00:00:00 2001 From: Tran Hoang Huan <121786621+tranh0anghuan@users.noreply.github.com> Date: Fri, 25 Sep 2026 11:41:36 +0700 Subject: [PATCH 23/34] Feat/tours new users only test (#233) * feat(env): tours_new_user_since cutoff for contextual tours Expose myDrumee.tours_new_user_since (unix seconds) as a platform flag. The client compares it with the account's entity.ctime so contextual tutorial tours are offered to new accounts only. 0/absent keeps today's behaviour (every account eligible), so deploy order is free. Co-Authored-By: Claude Opus 5.5 (1M context) * docs: guide to enable the tours_new_user_since cutoff Co-Authored-By: Claude Opus 5.5 (1M context) --------- Co-authored-by: Drumee Dev Co-authored-by: Claude Opus 5.5 (1M context) --- docs/tours-new-user-cutoff.md | 181 ++++++++++++++++++++++++++++++++++ service/lib/env.js | 7 ++ 2 files changed, 188 insertions(+) create mode 100644 docs/tours-new-user-cutoff.md diff --git a/docs/tours-new-user-cutoff.md b/docs/tours-new-user-cutoff.md new file mode 100644 index 0000000..4f7da15 --- /dev/null +++ b/docs/tours-new-user-cutoff.md @@ -0,0 +1,181 @@ +# Enable the new-user cutoff for tutorial tours (production) + +**Target:** https://app.drumee.com/-/#/desk (the `main` endpoint) +**Goal:** contextual tutorial tours (desk workspace tour; window migrate, chat, +meeting, task and share tours) are shown to accounts created **after** the +release only. Accounts that existed before it never see them. + +How it works: the server reads `tours_new_user_since` (unix seconds) from +`myDrumee.json` and sends it to the browser in `yp.get_env`. The UI compares it +with the account's creation time (`entity.ctime`) and offers no tour when the +account is older. No database change is involved. + +Explicit requests still work for everyone: `?tutorial=` in the URL and +**Get help → Product Tour**. + +--- + +## 0. State of production when this guide was written (2026-09-25) + +| Check | Value | +|---|---| +| `contextual_tours` | `1` (tours are on) | +| `tours_new_user_since` in `yp.get_env` | **absent** — server code not deployed | +| UI bundle contains the check | **no** — UI code not deployed | + +So the code must be deployed first (step 1). Adding the key alone does nothing. + +--- + +## 1. Deploy the code + +Both changes are on branch `feat/tours-new-users-only`: + +| Repo | Commit | File | +|---|---|---| +| server-team | `efda7b8` | `service/lib/env.js` | +| ui-team | `f6bd2843` | `src/drumee/libs/tutorial-tours.js` | + +Merge them through the normal `preview` → production release, and deploy +**server-team** and **ui-team** to the `main` endpoint. + +Order does not matter: the server sends `0` while the key is missing, and `0` +means "no cutoff" — today's behaviour. + +--- + +## 2. Pick the cutoff + +Use the moment the release goes live. On the server, right after the deploy: + +```bash +date +%s # e.g. 1790306573 +date -u -d @$(date +%s) # human-readable, for the record +``` + +Every account created **before** this second is treated as old. +Do not pick an earlier date: recent sign-ups who have not seen the tours yet +would lose them. + +--- + +## 3. Find the config file the `main` service reads + +On production, as a user with sudo: + +```bash +# The service process for the main endpoint +ps -eo pid,lstart,args | grep "server/main/service.js" | grep -v grep + +# The config is loaded by loadSysEnv(); default path: +ls -l /etc/drumee/conf.d/myDrumee.json + +# Check the main runtime does not use a chroot (a per-endpoint copy, like +# stage's conf.d/liam/myDrumee.json). No chroot argument = the default path. +grep -n "loadSysEnv" /srv/drumee/runtime/server/main/index.js /srv/drumee/runtime/server/main/configs.js +ls -d /etc/drumee/conf.d/*/ +``` + +If a per-endpoint directory exists for `main`, edit that file instead. +Note: `/etc/drumee/conf.d/myDrumee.json` is usually **shared** by every endpoint +on the box. Adding the key is harmless for endpoints running older code — they +ignore it. + +--- + +## 4. Add the key + +```bash +F=/etc/drumee/conf.d/myDrumee.json +TS=1790306573 # <- the value from step 2 + +# Back up, keeping owner and mode +sudo cp -p "$F" "$F.bak-preToursCutoff" + +# Add or replace the key without hand-editing the JSON +sudo python3 - "$F" "$TS" <<'EOF' +import json, sys +path, ts = sys.argv[1], int(sys.argv[2]) +data = json.load(open(path)) +data["tours_new_user_since"] = ts +open(path, "w").write(json.dumps(data, indent=2)) +EOF + +# Must print the key, and the file must still be valid JSON +python3 -m json.tool "$F" | grep -n "contextual_tours\|tours_new_user_since" +ls -l "$F" # owner should still be www-data +``` + +The result should contain: + +```json + "contextual_tours": 1, + "tours_new_user_since": 1790306573 +``` + +--- + +## 5. Restart the service + +`myDrumee.json` is read **only at startup**. + +```bash +sudo drumee restart main/service +ps -eo pid,lstart,args | grep "server/main/service.js" | grep -v grep # start time must be after step 4 +``` + +--- + +## 6. Verify + +From any machine, no login needed: + +```bash +# 1. The server sends the cutoff +curl -s "https://app.drumee.com/-/service/yp.get_env" | python3 -c \ + "import json,sys; d=json.load(sys.stdin); d=d.get('data',d); p=d['platform']; print(p.get('contextual_tours'), p.get('tours_new_user_since'))" +# expected: 1 1790306573 + +# 2. The live UI bundle has the check +M=$(curl -s "https://app.drumee.com/-/" | grep -o 'main-[0-9a-f]*\.js' | head -1) +curl -s "https://app.drumee.com/-/app/$M" | grep -c tours_new_user_since +# expected: 1 or more +``` + +In the browser (use a private window — the browser keeps its own per-account +record of seen tours): + +1. **Old account** (created before the cutoff): open a workspace, then Files, + Chat, Meet, Tasks and Manage access. **No tour appears.** +2. Same account with `?tutorial=chat` in the URL: the chat tour **does** appear. +3. **New account** (sign up after the cutoff): tours appear as before. + +Tabs that were open before the UI deploy keep the old code until a full reload. + +--- + +## Rollback + +```bash +F=/etc/drumee/conf.d/myDrumee.json +sudo cp -p "$F.bak-preToursCutoff" "$F" +sudo drumee restart main/service +``` + +To switch the cutoff off without restoring the file, set +`"tours_new_user_since": 0` and restart. `0` means every account is eligible. + +--- + +## Troubleshooting + +| Symptom | Cause | +|---|---| +| `yp.get_env` shows no `tours_new_user_since` | Server code not deployed to `main` | +| `yp.get_env` shows `0` | Key missing from the file the service reads, or service not restarted | +| Old accounts still get tours | UI not deployed, or the tab was not reloaded | +| New accounts get no tours | Cutoff set in the future, or `contextual_tours` is `0` | + +Stage reference: this was done on 2026-09-25 for `https://drumee.in/-/huan/` +with `tours_new_user_since = 1790306573` in the shared +`/etc/drumee/conf.d/myDrumee.json` (backup `myDrumee.json.bak-preToursCutoff`). diff --git a/service/lib/env.js b/service/lib/env.js index d4ce40d..708ae51 100644 --- a/service/lib/env.js +++ b/service/lib/env.js @@ -153,6 +153,13 @@ function platform() { // single post-signup tour and no trigger fires, so this is a true kill // switch — the client writes nothing while it is 0. platform.contextual_tours = global.myDrumee.contextual_tours ? 1 : 0; + // Unix seconds. Contextual tours are for NEW users only: an account whose + // entity.ctime (get_user → data.user.ctime) is older than this is never + // offered one. The client compares the two, so no backfill of + // tutorials_seen is needed for existing accounts. 0/absent = no cutoff, + // every account is eligible — today's behaviour, which makes the rollout + // order free: ship, then set the date in myDrumee.json. + platform.tours_new_user_since = ~~global.myDrumee.tours_new_user_since; platform.cdnHost = global.myDrumee.cdnHost; platform.version = global.VERSION; platform.TfaMethods = TfaMethods; From 257a32150a031fe7f5bea3a701c0949b18ec46a5 Mon Sep 17 00:00:00 2001 From: Aaron Vu Date: Fri, 25 Sep 2026 13:45:30 +0700 Subject: [PATCH 24/34] fix(channel): show only current readers in chat read receipts metadata._seen_ never forgets a uid, so a member who left, was removed, lost an expired grant or deleted the account kept appearing as a seen avatar in the workspace chat. channel.messages and channel.file_thread_messages now prune _seen_ to the hub's current readers (channel_reader_ids) before responding and broadcasting. The lookup fails open, so a hub without the procedure still lists its chat. --- service/lib/seen-readers.js | 79 +++++++++++++++++++++++++++++++++++++ service/private/channel.js | 4 ++ test/seen-readers.test.js | 63 +++++++++++++++++++++++++++++ 3 files changed, 146 insertions(+) create mode 100644 service/lib/seen-readers.js create mode 100644 test/seen-readers.test.js diff --git a/service/lib/seen-readers.js b/service/lib/seen-readers.js new file mode 100644 index 0000000..0aafc1e --- /dev/null +++ b/service/lib/seen-readers.js @@ -0,0 +1,79 @@ +/** + * @license + * Copyright 2024 Thidima SA. All Rights Reserved. + * Licensed under the GNU AFFERO GENERAL PUBLIC LICENSE, Version 3. + * https://www.gnu.org/licenses/agpl-3.0.html + */ + +/** + * Read receipts (`metadata._seen_`, a `{uid: ts}` map) only ever accumulate: + * nothing removes a uid when that person leaves the hub, is removed, loses an + * expired grant or deletes the account. The chat renders every key as a + * "seen" avatar, so such a former reader kept showing in a workspace they are + * no longer part of. + * + * Instead of rewriting history, the list services prune `_seen_` at read time + * to the hub's current readers (`channel_reader_ids`). The stored row is left + * untouched, so a returning member shows up again exactly where they read. + */ + +/** + * @param {object} db hub database handle (`this.db`) + * @returns {Promise>} uids that can read this hub's chat now + */ +async function currentReaders(db) { + const rows = await db.await_proc("channel_reader_ids"); + const list = Array.isArray(rows) ? rows : rows ? [rows] : []; + return new Set(list.map((r) => `${r.uid}`)); +} + +/** + * Drop every `_seen_` key that is not a current reader. Keeps the metadata's + * shape: a JSON string stays a string, an object is edited in place. + * @param {object[]} messages rows from a channel list procedure — mutated + * @param {Set} readers from currentReaders() + * @returns {object[]} the same `messages` + */ +function pruneSeen(messages, readers) { + for (const message of messages || []) { + if (!message || !message.metadata) continue; + const isString = typeof message.metadata === "string"; + let md; + try { + md = isString ? JSON.parse(message.metadata) : message.metadata; + } catch (e) { + continue; + } + const seen = md && md._seen_; + if (!seen || typeof seen !== "object") continue; + let changed = false; + for (const uid of Object.keys(seen)) { + if (!readers.has(`${uid}`)) { + delete seen[uid]; + changed = true; + } + } + if (changed && isString) message.metadata = JSON.stringify(md); + } + return messages; +} + +/** + * Prune `messages` to the hub's current readers. Fails open: a hub whose + * schema does not carry `channel_reader_ids` yet still lists its chat, with + * `_seen_` as stored, rather than failing the whole list. + * @param {object} ctx service handler (`this`: needs `db`, `warn`) + * @param {object[]} messages mutated in place + * @returns {Promise} the same `messages` + */ +async function pruneToCurrentReaders(ctx, messages) { + if (!messages || !messages.length) return messages; + try { + return pruneSeen(messages, await currentReaders(ctx.db)); + } catch (e) { + if (ctx.warn) ctx.warn("[seen-readers] reader lookup failed", e && e.message); + return messages; + } +} + +module.exports = { currentReaders, pruneSeen, pruneToCurrentReaders }; diff --git a/service/private/channel.js b/service/private/channel.js index 427cff4..fafcb8b 100644 --- a/service/private/channel.js +++ b/service/private/channel.js @@ -22,6 +22,7 @@ const { Entity, MfsTools } = require("@drumee/server-core"); const { remove_node, move_node } = MfsTools; const { copyNodeStorage } = require("./_node-storage"); const { stampAuthorIdentity } = require("../lib/message-author"); +const { pruneToCurrentReaders } = require("../lib/seen-readers"); const { movePlanRows } = require("./_move-plan"); const { memberCan, CAN_CHAT } = require("../lib/member-capability"); const {admit: admitMobilePush} = require('../lib/mobile-push'); @@ -212,6 +213,8 @@ class __private_channel extends Entity { this.uid, ); } + // Former members stay in _seen_ forever; show only current readers. + await pruneToCurrentReaders(this, messages); let dest = await this.yp.await_proc("entity_sockets", hub_id); dest = toArray(dest).filter((e) => { return e.uid != this.uid; @@ -1896,6 +1899,7 @@ class __private_channel extends Entity { ); } } + await pruneToCurrentReaders(this, data); this.output.list(data); } diff --git a/test/seen-readers.test.js b/test/seen-readers.test.js new file mode 100644 index 0000000..90ae602 --- /dev/null +++ b/test/seen-readers.test.js @@ -0,0 +1,63 @@ +// test/seen-readers.test.js +const assert = require("assert"); + +let passed = 0, failed = 0; +async function test(name, fn) { + try { await fn(); console.log(` ok ${name}`); passed++; } + catch (e) { console.log(` FAIL ${name}: ${e.message}`); failed++; } +} + +const { pruneSeen, pruneToCurrentReaders } = require("../service/lib/seen-readers"); + +(async () => { + await test("drops readers who are no longer in the hub, keeps string metadata a string", () => { + const rows = [{ metadata: JSON.stringify({ _seen_: { a: 1, gone: 2 }, _delivered_: { a: 1 } }) }]; + pruneSeen(rows, new Set(["a"])); + assert.strictEqual(typeof rows[0].metadata, "string"); + const md = JSON.parse(rows[0].metadata); + assert.deepStrictEqual(md._seen_, { a: 1 }); + assert.deepStrictEqual(md._delivered_, { a: 1 }); + }); + + await test("leaves a row untouched when every reader is current", () => { + const raw = '{"_seen_":{"a":1},"x":"keep"}'; + const rows = [{ metadata: raw }]; + pruneSeen(rows, new Set(["a"])); + assert.strictEqual(rows[0].metadata, raw); + }); + + await test("edits object metadata in place and skips rows without _seen_ or bad JSON", () => { + const rows = [{ metadata: { _seen_: { a: 1, b: 2 } } }, { metadata: "{bad" }, { metadata: null }, {}]; + pruneSeen(rows, new Set(["b"])); + assert.deepStrictEqual(rows[0].metadata._seen_, { b: 2 }); + assert.strictEqual(rows[1].metadata, "{bad"); + }); + + await test("reads the reader set from channel_reader_ids", async () => { + const calls = []; + const ctx = { db: { await_proc: (...a) => { calls.push(a); return Promise.resolve([{ uid: "a" }]); } } }; + const rows = [{ metadata: '{"_seen_":{"a":1,"b":2}}' }]; + await pruneToCurrentReaders(ctx, rows); + assert.deepStrictEqual(calls, [["channel_reader_ids"]]); + assert.deepStrictEqual(JSON.parse(rows[0].metadata)._seen_, { a: 1 }); + }); + + await test("fails open when the procedure is missing", async () => { + const warns = []; + const ctx = { warn: (...a) => warns.push(a), db: { await_proc: () => Promise.reject(new Error("PROCEDURE does not exist")) } }; + const raw = '{"_seen_":{"a":1}}'; + const rows = [{ metadata: raw }]; + await pruneToCurrentReaders(ctx, rows); + assert.strictEqual(rows[0].metadata, raw); + assert.strictEqual(warns.length, 1); + }); + + await test("does not query for an empty list", async () => { + let called = false; + await pruneToCurrentReaders({ db: { await_proc: () => { called = true; } } }, []); + assert.strictEqual(called, false); + }); + + console.log(`\n${passed} passed, ${failed} failed`); + process.exit(failed ? 1 : 0); +})(); From f3d763f175dd1e162a454eac2d3a124326250026 Mon Sep 17 00:00:00 2001 From: EddyOne81 Date: Fri, 25 Sep 2026 05:08:57 -0700 Subject: [PATCH 25/34] fix(media): unzip and server import write canonical media paths Both workers built parent_path/file_path with path.join. At a hub root join("", "") is ".", so an unzipped folder was stored as "." / "cye" and its children as "cye" / "cye/x.pdf". Exact file_path lookups and node_id_from_path then missed every node inside it: uploads into the folder landed at the workspace root, mkdir -p failed, and URLs came out as "/Hubcye/x.pdf". Into a subfolder, parent_path lost its trailing slash. Paths now come from service/lib/mfs-path.js in the shape parent_path() and filepath() return ("/cye", "/cye/", "/cye/x.pdf"). A destination that still holds a legacy relative path is normalised against the hub root. Folders are booked at 0 bytes like media.make_dir, not 1024. serverimport also stops writing "name.undefined" for top-level files. Co-Authored-By: Claude Opus 5.5 --- offline/media/serverimport.js | 38 ++++++++------------ offline/media/unzip.js | 13 ++++--- offline/test/mfs-path.test.js | 68 +++++++++++++++++++++++++++++++++++ service/lib/mfs-path.js | 64 +++++++++++++++++++++++++++++++++ 4 files changed, 153 insertions(+), 30 deletions(-) create mode 100644 offline/test/mfs-path.test.js create mode 100644 service/lib/mfs-path.js diff --git a/offline/media/serverimport.js b/offline/media/serverimport.js index a699910..8072331 100755 --- a/offline/media/serverimport.js +++ b/offline/media/serverimport.js @@ -23,6 +23,7 @@ const shell = require("shelljs"); const { readdirSync, statSync, existsSync } = require("fs"); const { parse, join } = require("path"); const Jsonfile = require("jsonfile"); +const { childPaths, nodeFolder } = require("../../service/lib/mfs-path"); const { Attr, getFileinfo, RedisStore, Mariadb, Offline, Cache, toArray, sysEnv, uniqueId } = require('@drumee/server-essentials'); @@ -119,11 +120,10 @@ class __offline_media_import extends Offline { node.mimetype = info.mimetype; node.lvl = lvl + 1; - node.parent_path = join(parent_path, ""); - node.file_path = join( + Object.assign(node, childPaths( parent_path, - node.user_filename + "." + node.extension - ); + node.extension ? `${node.user_filename}.${node.extension}` : node.user_filename + )); node.source = join(absolute, ""); node.destination = join(home_dir, node.id); @@ -134,19 +134,18 @@ class __offline_media_import extends Offline { ); if (statSync(absolute).isDirectory()) { - node.filesize = 1024; + node.filesize = 0; node.extension = ""; node.category = "folder"; node.mimetype = ""; node.lvl = lvl + 1; - node.parent_path = join(parent_path, ""); - node.file_path = join(parent_path, node.user_filename); + Object.assign(node, childPaths(parent_path, node.user_filename)); this.nodes.push(node); await this.getFilesRecursively( absolute, node.id, node.lvl, - join(node.parent_path, node.user_filename), + node.file_path, home_dir ); } else { @@ -209,14 +208,11 @@ class __offline_media_import extends Offline { node.category = info.category; node.mimetype = info.mimetype; node.lvl = 0; - dest_attr.parent_path = dest_attr.parent_path || ""; - dest_attr.filename = dest_attr.filename || ""; - node.parent_path = join(dest_attr.parent_path, dest_attr.filename); - node.file_path = join( - dest_attr.parent_path, - dest_attr.filename, - node.user_filename + "." + node.ext - ); + const destFolder = nodeFolder(dest_attr); + Object.assign(node, childPaths( + destFolder, + node.extension ? `${node.user_filename}.${node.extension}` : node.user_filename + )); node.source = join(absolute, ""); node.destination = join(dest_attr.home_dir, node.id); node.destination_file = join( @@ -226,15 +222,11 @@ class __offline_media_import extends Offline { ); if (statSync(absolute).isDirectory()) { - node.filesize = 1024; + node.filesize = 0; node.extension = ""; node.category = "folder"; node.mimetype = ""; - node.file_path = join( - dest_attr.parent_path, - dest_attr.filename, - node.user_filename - ); + Object.assign(node, childPaths(destFolder, node.user_filename)); node.source = ""; node.destination = ""; node.destination_file = ""; @@ -243,7 +235,7 @@ class __offline_media_import extends Offline { absolute, node.id, node.lvl, - join(node.parent_path, node.user_filename), + node.file_path, dest_attr.home_dir ); } else { diff --git a/offline/media/unzip.js b/offline/media/unzip.js index bdde856..01ea0fe 100755 --- a/offline/media/unzip.js +++ b/offline/media/unzip.js @@ -56,10 +56,11 @@ const { const { extract, MAX_ENTRIES, MAX_FILENAME, MAX_FILE_PATH, } = require("../../service/lib/archive"); +const { childPaths, nodeFolder } = require("../../service/lib/mfs-path"); const FOLDER = "folder"; -/** Folders are booked at the same nominal size serverimport books them at. */ -const FOLDER_SIZE = 1024; +/** Folders carry no bytes of their own, as media.make_dir writes them. */ +const FOLDER_SIZE = 0; class __offline_media_unzip extends Offline { /** @@ -239,7 +240,6 @@ class __offline_media_unzip extends Offline { "mfs_unique_filename", dest.id, baseName, ""); const folderName = (unique && unique.user_filename) || baseName; - const destParentPath = join(dest.parent_path || "", dest.filename || ""); const rootNode = { id: uniqueId(8, "hex"), parent_id: dest.id, @@ -249,8 +249,7 @@ class __offline_media_unzip extends Offline { category: FOLDER, filesize: FOLDER_SIZE, lvl: 0, - parent_path: destParentPath, - file_path: join(destParentPath, folderName), + ...childPaths(nodeFolder(dest), folderName), source: "", destination: "", destination_file: "", @@ -442,7 +441,7 @@ class __offline_media_unzip extends Offline { name = this.uniqueSibling(seen, parent.id, name, extension); const leaf = extension ? `${name}.${extension}` : name; - const filePath = join(parent.file_path, leaf); + const { parent_path, file_path: filePath } = childPaths(parent.file_path, leaf); if (filePath.length > MAX_FILE_PATH) { // Deeper than the column can record. Skipping the branch is the only // honest option: a truncated file_path would either collide with a @@ -460,7 +459,7 @@ class __offline_media_unzip extends Offline { category: isDir ? FOLDER : (info.category || "other"), filesize: isDir ? FOLDER_SIZE : st.size, lvl, - parent_path: parent.file_path, + parent_path, file_path: filePath, source: isDir ? "" : absolute, destination: isDir ? "" : join(homeDir, ""), diff --git a/offline/test/mfs-path.test.js b/offline/test/mfs-path.test.js new file mode 100644 index 0000000..a762bd9 --- /dev/null +++ b/offline/test/mfs-path.test.js @@ -0,0 +1,68 @@ +#!/usr/bin/env node +// +// mfs-path.test.js — path columns written by unzip / serverimport. +// +// node offline/test/mfs-path.test.js +// +// Both workers bulk-insert through mfs_import, which stores parent_path and +// file_path exactly as given. They used to build them with path.join, which +// gave "." / "cye" / "cye/x.pdf" at a hub root; exact file_path lookups and +// node_id_from_path then failed for everything inside the unzipped folder. +// The expected values below are what parent_path() / filepath() return. +// +// Exit code 0 = all pass, 1 = any failure. + +const assert = require('assert'); +const { childPaths, folderPath, nodeFolder } = require('../../service/lib/mfs-path'); + +let pass = 0; +let fail = 0; + +function check(name, fn) { + try { + fn(); + pass++; + console.log(` ok ${name}`); + } catch (e) { + fail++; + console.log(` FAIL ${name}\n ${e.message}`); + } +} + +// mfs_access_node on a hub's home: file_path "/", parent_path "", filename "". +const HOME = { ownpath: '/', parent_path: '', filename: '' }; + +check('unzip at hub root gives an absolute folder', () => { + assert.deepStrictEqual(childPaths(nodeFolder(HOME), 'cye'), + { parent_path: '/', file_path: '/cye' }); +}); + +check('child of that folder', () => { + assert.deepStrictEqual(childPaths('/cye', 'x.pdf'), + { parent_path: '/cye/', file_path: '/cye/x.pdf' }); +}); + +check('unzip into a subfolder keeps the trailing slash on parent_path', () => { + assert.deepStrictEqual(childPaths(nodeFolder({ ownpath: '/ws/sub' }), 'cye'), + { parent_path: '/ws/sub/', file_path: '/ws/sub/cye' }); +}); + +check('destination still holding a legacy relative path', () => { + assert.deepStrictEqual(childPaths(nodeFolder({ ownpath: 'cye/in' }), 'z'), + { parent_path: '/cye/in/', file_path: '/cye/in/z' }); +}); + +check('no ownpath: falls back to parent_path + filename', () => { + assert.strictEqual(nodeFolder({ parent_path: '', filename: '' }), '/'); + assert.strictEqual(nodeFolder({ parent_path: '/ws/', filename: 'sub' }), '/ws/sub'); +}); + +check('"." and duplicate slashes collapse, dot-names survive', () => { + assert.strictEqual(folderPath('.'), '/'); + assert.strictEqual(folderPath('//a//b/'), '/a/b'); + assert.deepStrictEqual(childPaths('/a', '.git'), + { parent_path: '/a/', file_path: '/a/.git' }); +}); + +console.log(`\n${pass} passed, ${fail} failed`); +process.exit(fail ? 1 : 0); diff --git a/service/lib/mfs-path.js b/service/lib/mfs-path.js new file mode 100644 index 0000000..0a8d579 --- /dev/null +++ b/service/lib/mfs-path.js @@ -0,0 +1,64 @@ +/** + * @license + * Copyright 2024 Thidima SA. All Rights Reserved. + * Licensed under the GNU AFFERO GENERAL PUBLIC LICENSE, Version 3 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.gnu.org/licenses/agpl-3.0.html + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * ============================================================================= + */ + +/** + * The path columns of `media`, in the exact shape the SQL side writes them + * (parent_path() / filepath()): `file_path` is absolute ("/", "/a/b.pdf") and + * `parent_path` is the parent's location WITH a trailing slash ("/", "/a/"). + * + * Workers that bulk-insert through mfs_import build these in JS, and + * path.join is the wrong tool for it: join("", "") is ".", and it never adds + * the leading or trailing slash. Rows written that way are invisible to every + * exact `file_path = ?` lookup and to node_id_from_path, which sends uploads + * into such a folder to the hub root instead. + */ +const { posix } = require("path"); + +/** + * Normalise a folder location to "/" or "/a/b". Also repairs the relative + * form older imports stored ("." or "a/b"), which is always relative to the + * hub root. + */ +function folderPath(p) { + const parts = String(p == null ? "" : p) + .split("/") + .filter((s) => s !== "" && s !== "."); + return `/${parts.join("/")}`; +} + +/** + * Path columns of an entry named `leaf` inside the folder at `dir`. + */ +function childPaths(dir, leaf) { + const base = folderPath(dir); + return { + parent_path: base === "/" ? "/" : `${base}/`, + file_path: posix.join(base, leaf), + }; +} + +/** + * Location of the destination node returned by mfs_access_node. `ownpath` is + * its raw file_path; older callers only had parent_path + filename. + */ +function nodeFolder(node) { + if (!node) return "/"; + if (node.ownpath != null && node.ownpath !== "") return folderPath(node.ownpath); + return folderPath(posix.join(node.parent_path || "", node.filename || "")); +} + +module.exports = { folderPath, childPaths, nodeFolder }; From 44d1d0fde4661d865e5be7c991152ddb52d775af Mon Sep 17 00:00:00 2001 From: "phamtobao@gmail.com" Date: Sat, 26 Sep 2026 02:24:56 +0400 Subject: [PATCH 26/34] fix(media): trash every node a batch request names, not only the ACL reference node --- service/private/media.js | 71 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 68 insertions(+), 3 deletions(-) diff --git a/service/private/media.js b/service/private/media.js index 2d69341..f713655 100644 --- a/service/private/media.js +++ b/service/private/media.js @@ -57,7 +57,7 @@ const { /** filecap.category for every archive extension — what media.filetype carries. */ const ARCHIVE_CATEGORY = "zip"; const { stringify } = JSON; -const { isEmpty, isString, values } = require("lodash"); +const { isEmpty, isString, isArray, isObject, values } = require("lodash"); const { join, resolve, basename, extname, dirname } = require("path"); const { existsSync, readFileSync, writeFileSync, readdirSync, statSync, copyFileSync, mkdirSync, renameSync, cpSync, rmSync } = require("fs"); const { writeFileSync: writeJson } = require("jsonfile"); @@ -2508,10 +2508,75 @@ class __private_media extends Media { * * @returns */ + /** + * Every node a trash request names, checked one by one. + * + * The ACL layer treats `nid` as a REFERENCE: with an array it grants the + * first entry only, so source_nodes() hands back one node however many the + * client sent and a multi-select "Move to trash" removed exactly one file + * (the desk used to hide this by sending one request per tile, which then + * collided in the DB). This reads the list the client actually sent, in any + * of the shapes the service has ever accepted, and looks every node up on + * its hub as this user: a node that is gone already is skipped, one the user + * may not delete refuses the whole batch, a locked one too. Cached on the + * heap because pre_trash and trash both need it. + * + * @returns {Promise|null>} null after + * answering the client with the refusal + */ + async _trashTargets() { + if (isArray(this.heap.nodes)) return this.heap.nodes; + const currentHub = this.hub.get(Attr.id); + let raw = this.input.get(Attr.nid); + if (isString(raw)) { + try { raw = JSON.parse(raw); } catch (e) { /* a plain node id */ } + } + const wanted = []; + const add = (nid, hub_id) => { + if (nid == null || nid === "") return; + wanted.push({ nid: String(nid), hub_id: String(hub_id || currentHub) }); + }; + const addEntry = (o) => { + if (isString(o)) return add(o, currentHub); + if (!isObject(o)) return; + if (isArray(o.nid)) return o.nid.forEach((id) => add(id, o.hub_id)); + add(o.nid || o.id, o.hub_id); + }; + if (isArray(raw)) raw.forEach(addEntry); else addEntry(raw); + if (!wanted.length) { + this.heap.nodes = this.source_nodes(); + return this.heap.nodes; + } + + const seen = new Set(); + const targets = []; + for (const t of wanted) { + const key = `${t.hub_id}:${t.nid}`; + if (seen.has(key)) continue; + seen.add(key); + const hub_db = await this.yp.await_func("get_db_name", t.hub_id); + if (!hub_db) continue; + const node = await this.yp.await_proc(`${hub_db}.mfs_access_node`, this.uid, t.nid); + if (!node || !node.id) continue; // already gone: nothing to refuse + if (!(Number(node.privilege) & Permission.DELETE)) { + this.warn(`trash refused: no delete right on ${t.hub_id}/${t.nid}`); + this.exception.user("PERMISSION_DENIED"); + return null; + } + if (node.status === "locked") { + this.exception.user(LOCKED); + return null; + } + targets.push(t); + } + this.heap.nodes = targets; + return targets; + } + async pre_trash() { const src = this.source_granted(Attr.all); - this.heap.nodes = this.heap.nodes || this.source_nodes(); //JSON.parse(this.src.args); + if (!(await this._trashTargets())) return; this.heap.srcgrantlst = []; let granted = []; let tnode; @@ -2556,7 +2621,7 @@ class __private_media extends Media { * @returns */ async trash() { - this.heap.nodes = this.heap.nodes || this.source_nodes(); //JSON.parse(this.src.args); + if (!(await this._trashTargets())) return; this.heap.srcgrantlst = []; let granted = []; let node; From bd1d67dd77eefe8d52f9a3547214893e6a8cf1a4 Mon Sep 17 00:00:00 2001 From: luongtrieuvy202 Date: Sat, 26 Sep 2026 21:02:22 +0700 Subject: [PATCH 27/34] fix(conference): stop announcing meetings that did not just start MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Users kept getting "X started a meeting" for meetings that were long under way, already over, or in workspaces they are not members of. - Announce conference.start to the workspace only when the joiner is the room's first, and once per meeting (Redis SET NX marker). A host rejoin, a media Retry, a member promoted to host in a hostless room, and the first join after a server restart wiped yp.conference no longer re-announce. - The marker is a heartbeat: 5 min TTL re-armed by every participant's 30 s diagnostics ping, so a dead room frees its workspace within minutes and never swallows the next genuine meeting's announcement. - Send it to workspace members only ('*' grant, live, not expired). entity_sockets matched any permission row, e.g. a single shared file. entity_sockets itself is unchanged (every hub broadcast uses it). - When a meeting room empties (conference.leave or a dropped socket), flip its still-live "started a meeting" chat cards to ended and clear the marker. The card used to be flipped only by a clean client teardown, so a closed/crashed tab left it offering "Join meeting" forever, and joining from it started a new meeting in the clicker's name. The dropped-socket path now also clears the duration-cap start, like the clean leave does. Everything fails open: no Redis → first-joiner rule; unknown membership → previous recipient list; card cleanup errors never fail a leave. Co-Authored-By: Claude Opus 5.5 --- router/push/index.js | 17 +- service/conference.js | 51 +++++- service/lib/meeting-lifecycle.js | 286 +++++++++++++++++++++++++++++++ test/meeting-lifecycle.test.js | 237 +++++++++++++++++++++++++ 4 files changed, 586 insertions(+), 5 deletions(-) create mode 100644 service/lib/meeting-lifecycle.js create mode 100644 test/meeting-lifecycle.test.js diff --git a/router/push/index.js b/router/push/index.js index 842e119..104a7ef 100644 --- a/router/push/index.js +++ b/router/push/index.js @@ -18,6 +18,8 @@ const { const { isFunction, isEmpty, isString } = require("lodash"); const { Data, Input } = require("@drumee/server-core"); const Page = require("../../client/page"); +const { clearRoomStart } = require("../../service/lib/meeting-limit"); +const { onRoomEmptied } = require("../../service/lib/meeting-lifecycle"); const WATCHDOG_TIMER = 15000; @@ -191,7 +193,20 @@ class __websocket_router extends Logger { const recipients = remaining.filter( (p) => p && p.socket_id && p.socket_id !== socket_id ); - if (!recipients.length) continue; + if (!recipients.length) { + // LAST ONE OUT, the abrupt way — the tab was closed, crashed or lost + // its network, so no conference.leave (and no client-side card flip) + // will ever come. Do what the clean leave does for an emptied room: + // forget the duration-cap start and the start announcement, and turn + // the "started a meeting" card into "Meeting ended". Before this the + // card kept offering Join forever, and clicking it started a NEW + // meeting in the clicker's name. + if (r.type === "meeting") { + await clearRoomStart(r.room_id); + await onRoomEmptied(this.yp, { hub_id: r.hub_id, room_id: r.room_id }); + } + continue; + } // Same person still in the room on another socket — a second device, or // a reconnect that already landed. Announcing a leave here would drop a // participant who is very much still present, so say nothing: the row diff --git a/service/conference.js b/service/conference.js index ba8974e..c23c5ea 100644 --- a/service/conference.js +++ b/service/conference.js @@ -20,6 +20,13 @@ const { isArray, isEmpty, map } = require("lodash"); const __yp = require("./yp"); const { roomDeadline, clearRoomStart } = require("./lib/meeting-limit"); +const { + claimStartAnnouncement, + touchStartAnnouncement, + workspaceMemberIds, + onlyMembers, + onRoomEmptied, +} = require("./lib/meeting-lifecycle"); const { markFeatureUsage } = require("./lib/feature-usage"); class conference extends __yp { @@ -189,11 +196,28 @@ class conference extends __yp { const isFirstJoiner = attendees.length === 0; if ((user.role == "host" || isFirstJoiner) && user.uid == this.uid) { await this.inform({ recipients, payload }, "conference.start"); - if (room_type == Attr.meeting) { + // Tell the WORKSPACE only when the meeting actually starts. This branch is + // also entered by a host REjoining (reload, network blip, media Retry) and + // by a member promoted to host in a room whose host stepped out — both + // join a meeting that is already running, and announcing them is what + // popped "X started a meeting" at people for meetings long under way. + // isFirstJoiner rules those out; the per-room marker also rules out the + // first join after a server restart wiped yp.conference mid-call. See + // service/lib/meeting-lifecycle. + if ( + room_type == Attr.meeting && + isFirstJoiner && + (await claimStartAnnouncement(room_id)) + ) { try { const hub_id = this.hub.get(Attr.id); - const hubMembers = await this.yp.await_proc('entity_sockets', { hub_id, exclude: [socket_id] }); - if (hubMembers && toArray(hubMembers).length) { + // Members only: entity_sockets matches any permission row in the hub, + // including people who were only ever shared one file from it. + const hubMembers = onlyMembers( + await this.yp.await_proc('entity_sockets', { hub_id, exclude: [socket_id] }), + await workspaceMemberIds(this.db), + ); + if (hubMembers.length) { // `details` is mfs_node_attr(room_id) against THIS hub's db, but a // hub node lives in its owner's db — so for a meeting (room_id == // hub_id) it comes back empty and details.filename, which the @@ -227,7 +251,7 @@ class conference extends __yp { attendees: roster, joined: roster.length, }; - await RedisStore.sendData(this.payload(startPayload, { service: 'conference.start' }), toArray(hubMembers)); + await RedisStore.sendData(this.payload(startPayload, { service: 'conference.start' }), hubMembers); } } catch (e) { this.warn('conference.start: hub member notify failed', e && e.message); @@ -318,6 +342,16 @@ class conference extends __yp { let room_id = this.input.need(Attr.room_id); let socket_id = this.input.need(Attr.socket_id); let r = await this.yp.await_func('is_socket_bound', socket_id, this.session.sid()); + // Read BEFORE conference_leave deletes it: which workspace this room lives + // in and what kind of room it is, for the last-one-out cleanup below. The + // row can already be gone (released on disconnect, or wiped by a server + // restart) — then fall back to what the client sent. + const mine = toArray( + await this.yp.await_proc("conference_of_socket", socket_id) + ).find((c) => c && `${c.room_id}` === `${room_id}`); + const metadata = this.input.get(Attr.metadata) || {}; + const room_type = (mine && mine.type) || metadata.type || this.input.get(Attr.type); + const room_hub_id = (mine && mine.hub_id) || (this.hub && this.hub.get(Attr.id)); let remaining = await this.yp.await_proc("conference_leave", room_id, socket_id); if (remaining && !isArray(remaining)) remaining = [remaining]; const recipients = (remaining || []).filter( @@ -344,6 +378,12 @@ class conference extends __yp { // room, minus this caller — the same list the notify above uses — so // "nobody to tell" and "nobody left" are by definition the same fact. await clearRoomStart(room_id); + // Same fact, for the meeting itself: the next join is a NEW meeting (and + // may be announced again), and the "started a meeting" card must stop + // offering Join — the client flips it only from a clean teardown. + if (room_type == Attr.meeting) { + await onRoomEmptied(this.yp, { hub_id: room_hub_id, room_id }); + } } let peers; if (!r) { @@ -402,6 +442,9 @@ class conference extends __yp { line = `unserializable diagnostics: ${e && e.message}`; } this.debug("[conference.diag]", line); + // The ping doubles as the room's heartbeat: while anyone is still in it, + // its start stays announced (see service/lib/meeting-lifecycle). + await touchStartAnnouncement(room_id); if (event === "diag") { this.output.data(); return; diff --git a/service/lib/meeting-lifecycle.js b/service/lib/meeting-lifecycle.js new file mode 100644 index 0000000..8ce1af7 --- /dev/null +++ b/service/lib/meeting-lifecycle.js @@ -0,0 +1,286 @@ +/** + * Workspace-meeting lifecycle: WHEN a meeting counts as started (and is + * announced to the workspace), WHO that announcement reaches, and cleaning up + * the "X started a meeting" chat card when the room empties. + * + * WHY THIS EXISTS. Users kept getting "X started a meeting" for meetings that + * did not exist. Three server-side causes, each fixed here: + * + * 1. conference.start fired on every HOST join, not on the start. Since + * conference_join hands the host role to the first edit-tier joiner of a + * hostless room, a host who reloads, a network blip, a Retry on the + * blocked-media screen, or a member joining after the host stepped out + * each re-announced a meeting that had been running for an hour. + * → only the FIRST joiner announces, and only once per room lifetime + * (claimStartAnnouncement). + * + * 2. A server restart (socket_reset) wipes yp.conference while the Jitsi + * call carries on, so the next person to join a RUNNING meeting looked + * like its first joiner. The per-room Redis marker survives the restart, + * so that join is recognised as a continuation and not announced. + * + * 3. The announcement went to entity_sockets, which matches ANY row in the + * hub's permission table — single-file shares, links, expired or zeroed + * grants — so people outside the workspace were told about its meetings. + * → filtered to workspace-level members (workspaceMemberIds). NOT fixed + * inside entity_sockets itself: every hub broadcast (chat posts, file + * threads, …) goes through it, and those recipients are a separate + * question. + * + * And the chat card: it was flipped to "ended" only by the client, from the + * meeting window's teardown — which never runs when the last tab is closed, + * crashes or loses its network. The card then offered "Join meeting" forever, + * and clicking it STARTED a new meeting (the clicker is the first joiner), + * which re-announced to everyone. endLiveMeetingCards flips it server-side + * when the room empties, on both the clean leave and the dropped-socket path. + * + * `room_id` for a workspace meeting is the workspace node and is reused by + * every meeting the workspace holds — the marker is therefore cleared when the + * room empties, and otherwise lives only as long as the room shows signs of + * life (see ANNOUNCED_TTL_SEC). + * + * Nothing here throws: a failed announcement check or cleanup must never fail + * a join or a leave. + */ + +const { RedisStore, toArray } = require("@drumee/server-essentials"); + +const ANNOUNCED_KEY = "meeting:announced:"; + +/** + * How long an announcement marker outlives the last sign of life in its room. + * + * It is normally deleted when the room empties (forgetStartAnnouncement). The + * TTL covers the case where that never happens — a server restart wipes + * yp.conference, then everyone just closes their tab: no conference.leave, and + * the disconnect path finds no row to release. A long fixed TTL would then + * silently swallow the NEXT genuine meeting's announcement for hours. + * + * So the marker is a HEARTBEAT instead: short-lived, and re-armed by every + * participant's periodic diagnostics ping (conference.update event=diag, every + * 30 s per joined client — webrtc/room/jitsi.js DIAG_PERIOD_MS). That ping + * needs no conference row, so it keeps a running meeting marked even after a + * restart, and a dead meeting frees its room within this window. 5 min leaves + * room for background-tab timer throttling (down to ~1 call/min in Chrome). + * A client that never pings degrades to the first-joiner rule alone. + */ +const ANNOUNCED_TTL_SEC = 5 * 60; + +const MEETING_CARD_PREFIX = "[[MEETING:start:"; + +/** + * Take the right to announce this room's meeting. The first caller wins (SET + * NX); every later caller — a rejoin, a promoted host, a join after a server + * restart wiped the conference rows — loses until the room empties. + * + * Fails OPEN (true) when Redis is unavailable: that is the pre-fix behaviour + * (announce the first joiner), not silence. + * + * @param {String} roomId + * @returns {Promise} + */ +async function claimStartAnnouncement(roomId) { + if (!roomId) return true; + try { + const client = RedisStore.getClient(); + if (!client) return true; + const won = await client.set( + ANNOUNCED_KEY + roomId, + String(Math.floor(Date.now() / 1000)), + { NX: true, EX: ANNOUNCED_TTL_SEC }, + ); + return !!won; + } catch (e) { + return true; + } +} + +/** + * Someone is still in this room (their diagnostics ping arrived): push the + * marker's expiry out. EXPIRE on a missing key is a no-op, so this can never + * create a marker — only the first joiner's claim does. + * @param {String} roomId + */ +async function touchStartAnnouncement(roomId) { + if (!roomId) return; + try { + const client = RedisStore.getClient(); + if (!client) return; + await client.expire(ANNOUNCED_KEY + roomId, ANNOUNCED_TTL_SEC); + } catch (e) { } +} + +/** + * The room is empty: its next join is a new meeting again. + * @param {String} roomId + */ +async function forgetStartAnnouncement(roomId) { + if (!roomId) return; + try { + const client = RedisStore.getClient(); + if (!client) return; + await client.del(ANNOUNCED_KEY + roomId); + } catch (e) { } +} + +/** + * Workspace members: the account-wide ('*') grant with a live privilege — the + * same definition hub_get_members_by_type uses. A row on a single node (a + * shared file, a link, a chat attachment) does NOT make someone a member. + * + * Returns null when the answer is unknown (query failed, or came back empty, + * which for a workspace — it always has an owner row — means failure). The + * caller then keeps its unfiltered list rather than dropping everyone. + * + * @param {Object} db the hub's own connection (service `this.db`) + * @returns {Promise|null>} + */ +async function workspaceMemberIds(db) { + if (!db || typeof db.await_query !== "function") return null; + try { + const rows = toArray( + await db.await_query( + "SELECT DISTINCT entity_id FROM permission WHERE resource_id='*' " + + "AND permission > 0 AND (expiry_time = 0 OR expiry_time > UNIX_TIMESTAMP())" + ) + ).filter((r) => r && r.entity_id); + if (!rows.length) return null; + return new Set(rows.map((r) => String(r.entity_id))); + } catch (e) { + return null; + } +} + +/** + * Keep only the sockets of workspace members. Unknown membership → unchanged. + * @param {Array} sockets entity_sockets rows ({socket_id, uid}) + * @param {Set|null} members + */ +function onlyMembers(sockets, members) { + const list = toArray(sockets).filter(Boolean); + if (!members) return list; + return list.filter((s) => s.uid != null && members.has(String(s.uid))); +} + +function parseJson(raw) { + if (raw && typeof raw === "object") return raw; + if (typeof raw !== "string" || !raw) return null; + try { + return JSON.parse(raw); + } catch (e) { + return null; + } +} + +/** + * Is this chat row a still-live start card for `roomId`? Same matching rule as + * the client's window_meeting._findLiveMeetingCardId: the payload's room_id, + * or — for cards posted before room_id was carried — its nid (a workspace + * meeting's room_id IS its node id). + */ +function isLiveCardFor(row, roomId) { + if (!row || typeof row.message !== "string") return false; + const m = row.message.match(/^\[\[MEETING:start:([\s\S]*)\]\]$/); + if (!m) return false; + const payload = parseJson(m[1]) || {}; + const rid = payload.room_id != null ? payload.room_id : payload.nid; + if (rid == null || String(rid) !== String(roomId)) return false; + const md = parseJson(row.metadata); + return !(md && md.meeting_status === "ended"); +} + +/** + * The room just emptied: flip every still-live start card for it to "ended" + * and push the updated rows to the workspace, exactly as channel.meeting_end + * does when a client asks. Idempotent with that path — a client that also + * flips (clean teardown) re-writes the same value. + * + * Every live card, not only the newest: duplicates posted before this fix + * (restart / double-start) would otherwise keep offering "Join meeting". + * + * @param {Object} yp yp connection (reaches the hub DB by name) + * @param {Object} opt + * @param {String} opt.hub_id + * @param {String} opt.room_id + * @returns {Promise} cards flipped + */ +async function endLiveMeetingCards(yp, opt = {}) { + const { hub_id, room_id } = opt; + if (!yp || !hub_id || !room_id) return 0; + try { + const ent = toArray( + await yp.await_query("SELECT db_name FROM entity WHERE id=?", hub_id) + )[0]; + const db = ent && ent.db_name; + // Interpolated as an identifier, so it must be a plain name. + if (!db || !/^\w+$/.test(db)) return 0; + + const rows = toArray( + await yp.await_query( + `SELECT message_id, message, metadata FROM \`${db}\`.channel ` + + "WHERE message LIKE ? AND status = 'active'", + `${MEETING_CARD_PREFIX}%` + ) + ).filter((r) => isLiveCardFor(r, room_id)); + if (!rows.length) return 0; + + let recipients = null; + let flipped = 0; + for (const r of rows) { + // Same targeted write as hub channel_meeting_end: only meeting_status + // changes, every other metadata key is kept. + await yp.await_query( + `UPDATE \`${db}\`.channel SET metadata = JSON_SET(` + + "COALESCE(NULLIF(metadata, ''), '{}'), '$.meeting_status', 'ended') " + + "WHERE message_id = ?", + r.message_id + ); + const message = toArray( + await yp.await_query( + `SELECT * FROM \`${db}\`.channel WHERE message_id = ?`, + r.message_id + ) + )[0]; + if (!message) continue; + flipped++; + // key_id is what channel.meeting_end already sends (chat inbox rows find + // the conversation by it); hub_id is what the folder window's Start + // button checks before it will un-light "Join meeting". + message.key_id = hub_id; + message.hub_id = hub_id; + if (!recipients) { + recipients = toArray(await yp.await_proc("entity_sockets", { hub_id })); + } + await RedisStore.sendData( + { model: message, options: { service: "channel.meeting_end", keys: "*" } }, + recipients + ); + } + return flipped; + } catch (e) { + return 0; + } +} + +/** + * Everything "the room is now empty" means for a workspace meeting. + * @param {Object} yp + * @param {Object} opt { hub_id, room_id } + */ +async function onRoomEmptied(yp, opt = {}) { + await forgetStartAnnouncement(opt.room_id); + await endLiveMeetingCards(yp, opt); +} + +module.exports = { + ANNOUNCED_KEY, + ANNOUNCED_TTL_SEC, + claimStartAnnouncement, + touchStartAnnouncement, + forgetStartAnnouncement, + workspaceMemberIds, + onlyMembers, + isLiveCardFor, + endLiveMeetingCards, + onRoomEmptied, +}; diff --git a/test/meeting-lifecycle.test.js b/test/meeting-lifecycle.test.js new file mode 100644 index 0000000..378ea2c --- /dev/null +++ b/test/meeting-lifecycle.test.js @@ -0,0 +1,237 @@ +/** + * Cover for service/lib/meeting-lifecycle — the "X started a meeting" popup + * that users kept seeing for meetings that did not exist. + * + * 1. announce once per room lifetime (a rejoin / promoted host / post-restart + * join must not re-announce), kept alive by the diagnostics heartbeat, + * and fail open when Redis is down; + * 2. only workspace members ('*' grant) receive it, and an unknown member + * list never drops everyone; + * 3. an emptied room flips every still-live start card for THAT room, and + * leaves other rooms' cards and ended cards alone. + * + * Run: node test/meeting-lifecycle.test.js + */ +const assert = require("assert"); + +// In-memory Redis with the node-redis v4 calls the module uses. +const store = new Map(); +let redisDown = false; +const client = { + async set(k, v, opt = {}) { + if (redisDown) throw new Error("redis down"); + if (opt.NX && store.has(k)) return null; + store.set(k, v); + return "OK"; + }, + async del(k) { + if (redisDown) throw new Error("redis down"); + store.delete(k); + }, + expired: [], + async expire(k, sec) { + if (redisDown) throw new Error("redis down"); + this.expired.push([k, sec]); + return store.has(k) ? 1 : 0; + }, +}; +const sent = []; +require.cache[require.resolve("@drumee/server-essentials")] = { + exports: { + RedisStore: { + getClient: () => client, + sendData: async (payload, dest) => { sent.push({ payload, dest }); }, + }, + toArray: (v) => (Array.isArray(v) ? v : v == null ? [] : [v]), + }, +}; + +const lib = require("../service/lib/meeting-lifecycle"); + +const card = (message_id, payload, metadata) => ({ + message_id, + message: `[[MEETING:start:${JSON.stringify(payload)}]]`, + metadata: metadata === undefined ? null : JSON.stringify(metadata), +}); + +/** + * A yp stand-in that serves entity + one hub channel table and records writes. + * Answers like the real driver: a single row collapses to an object. + */ +function fakeYp(rows, { db_name = "hub_db" } = {}) { + const updates = []; + const byId = new Map(rows.map((r) => [r.message_id, { ...r }])); + return { + updates, + byId, + async await_query(sql, ...args) { + if (/FROM entity/.test(sql)) return db_name ? { db_name } : []; + if (/^UPDATE/.test(sql)) { + const r = byId.get(args[0]); + const md = JSON.parse((r && r.metadata) || "{}"); + md.meeting_status = "ended"; + if (r) r.metadata = JSON.stringify(md); + updates.push(args[0]); + return { affectedRows: 1 }; + } + if (/WHERE message LIKE/.test(sql)) return [...byId.values()]; + if (/WHERE message_id = \?/.test(sql)) return byId.get(args[0]) || []; + throw new Error("unexpected sql " + sql); + }, + async await_proc(name) { + assert.strictEqual(name, "entity_sockets"); + return [{ socket_id: "s1", uid: "u1" }, { socket_id: "s2", uid: "u2" }]; + }, + }; +} + +let failures = 0; +async function test(name, fn) { + try { await fn(); console.log(` ok ${name}`); } + catch (e) { failures++; console.log(` FAIL ${name}: ${e.message}`); } +} + +(async () => { + // ── announcement ────────────────────────────────────────────────────────── + await test("first claim wins, every later claim loses until the room empties", async () => { + store.clear(); + assert.strictEqual(await lib.claimStartAnnouncement("room1"), true); + // host reload / promoted host / first join after a server restart + assert.strictEqual(await lib.claimStartAnnouncement("room1"), false); + assert.strictEqual(await lib.claimStartAnnouncement("room1"), false); + // another workspace is independent + assert.strictEqual(await lib.claimStartAnnouncement("room2"), true); + await lib.forgetStartAnnouncement("room1"); + assert.strictEqual(await lib.claimStartAnnouncement("room1"), true, "next meeting announces again"); + }); + + await test("the marker carries a bounded TTL", async () => { + let seen; + const orig = client.set; + client.set = async (k, v, opt) => { seen = opt; return "OK"; }; + await lib.claimStartAnnouncement("room-ttl"); + client.set = orig; + assert.strictEqual(seen.NX, true); + assert.ok(seen.EX > 0 && seen.EX <= 24 * 3600, `EX=${seen.EX}`); + }); + + await test("heartbeat re-arms a live marker and can never create one", async () => { + store.clear(); + client.expired.length = 0; + await lib.touchStartAnnouncement("ghost"); + assert.strictEqual(store.has(lib.ANNOUNCED_KEY + "ghost"), false, "touch must not create"); + await lib.claimStartAnnouncement("live"); + await lib.touchStartAnnouncement("live"); + assert.deepStrictEqual(client.expired[1], [lib.ANNOUNCED_KEY + "live", lib.ANNOUNCED_TTL_SEC]); + // the TTL is a heartbeat window, not hours: a dead room frees up quickly + assert.ok(lib.ANNOUNCED_TTL_SEC <= 10 * 60, `TTL=${lib.ANNOUNCED_TTL_SEC}`); + // and long enough to survive background-tab throttling of a 30 s ping + assert.ok(lib.ANNOUNCED_TTL_SEC >= 2 * 60, `TTL=${lib.ANNOUNCED_TTL_SEC}`); + }); + + await test("fails open when Redis is unavailable (pre-fix behaviour, not silence)", async () => { + redisDown = true; + assert.strictEqual(await lib.claimStartAnnouncement("room3"), true); + await lib.forgetStartAnnouncement("room3"); // must not throw + await lib.touchStartAnnouncement("room3"); // must not throw + redisDown = false; + }); + + // ── recipients ──────────────────────────────────────────────────────────── + await test("only sockets of workspace members are kept", async () => { + const db = { await_query: async () => [{ entity_id: "u1" }, { entity_id: "u3" }] }; + const members = await lib.workspaceMemberIds(db); + const out = lib.onlyMembers( + [{ socket_id: "a", uid: "u1" }, { socket_id: "b", uid: "u2" }, { socket_id: "c", uid: "u3" }], + members, + ); + assert.deepStrictEqual(out.map((s) => s.socket_id), ["a", "c"]); + }); + + await test("a single-row member answer (collapsed object) still counts", async () => { + const db = { await_query: async () => ({ entity_id: "u1" }) }; + const members = await lib.workspaceMemberIds(db); + assert.ok(members && members.has("u1")); + }); + + await test("unknown membership keeps the unfiltered list instead of dropping everyone", async () => { + const sockets = [{ socket_id: "a", uid: "u1" }, { socket_id: "b", uid: "u2" }]; + for (const db of [ + { await_query: async () => [] }, + { await_query: async () => { throw new Error("db down"); } }, + null, + ]) { + const members = await lib.workspaceMemberIds(db); + assert.strictEqual(members, null); + assert.strictEqual(lib.onlyMembers(sockets, members).length, 2); + } + }); + + // ── card matching ───────────────────────────────────────────────────────── + await test("isLiveCardFor matches room_id, falls back to nid, skips ended / other rooms", () => { + assert.ok(lib.isLiveCardFor(card("m1", { room_id: "R", nid: "R" }), "R")); + assert.ok(lib.isLiveCardFor(card("m2", { nid: "R" }), "R"), "legacy card without room_id"); + assert.ok(!lib.isLiveCardFor(card("m3", { room_id: "X", nid: "X" }), "R")); + assert.ok(!lib.isLiveCardFor(card("m4", { room_id: "R" }, { meeting_status: "ended" }), "R")); + assert.ok(lib.isLiveCardFor(card("m5", { room_id: "R" }, { _seen_: {} }), "R"), "other metadata keys"); + assert.ok(!lib.isLiveCardFor({ message_id: "m6", message: "hello" }, "R")); + assert.ok(!lib.isLiveCardFor({ message_id: "m7", message: "[[MEETING:start:{not json]]" }, "R")); + }); + + // ── emptied room ────────────────────────────────────────────────────────── + await test("flips every live card of the emptied room and broadcasts each", async () => { + sent.length = 0; + const yp = fakeYp([ + card("live1", { room_id: "R", nid: "R" }), + card("live2", { nid: "R" }), // duplicate from before the fix + card("other", { room_id: "X", nid: "X" }), + card("done", { room_id: "R" }, { meeting_status: "ended" }), + ]); + const n = await lib.endLiveMeetingCards(yp, { hub_id: "H", room_id: "R" }); + assert.strictEqual(n, 2); + assert.deepStrictEqual(yp.updates.sort(), ["live1", "live2"]); + assert.strictEqual(sent.length, 2); + for (const { payload, dest } of sent) { + assert.strictEqual(payload.options.service, "channel.meeting_end"); + assert.strictEqual(payload.model.key_id, "H"); + assert.strictEqual(payload.model.hub_id, "H"); + assert.strictEqual(JSON.parse(payload.model.metadata).meeting_status, "ended"); + assert.strictEqual(dest.length, 2); + } + }); + + await test("nothing to flip → no write, no broadcast", async () => { + sent.length = 0; + const yp = fakeYp([card("other", { room_id: "X" })]); + assert.strictEqual(await lib.endLiveMeetingCards(yp, { hub_id: "H", room_id: "R" }), 0); + assert.strictEqual(yp.updates.length, 0); + assert.strictEqual(sent.length, 0); + }); + + await test("an unsafe or missing db name is refused", async () => { + for (const db_name of ["x`; DROP TABLE t; --", ""]) { + const yp = fakeYp([card("c", { room_id: "R" })], { db_name }); + assert.strictEqual(await lib.endLiveMeetingCards(yp, { hub_id: "H", room_id: "R" }), 0); + assert.strictEqual(yp.updates.length, 0); + } + }); + + await test("never throws on a DB failure", async () => { + const yp = { await_query: async () => { throw new Error("db down"); } }; + assert.strictEqual(await lib.endLiveMeetingCards(yp, { hub_id: "H", room_id: "R" }), 0); + await lib.onRoomEmptied(yp, { hub_id: "H", room_id: "R" }); + }); + + await test("onRoomEmptied re-arms the announcement AND flips the card", async () => { + store.clear(); + sent.length = 0; + await lib.claimStartAnnouncement("R"); + const yp = fakeYp([card("live", { room_id: "R" })]); + await lib.onRoomEmptied(yp, { hub_id: "H", room_id: "R" }); + assert.deepStrictEqual(yp.updates, ["live"]); + assert.strictEqual(await lib.claimStartAnnouncement("R"), true); + }); + + console.log(failures ? `\n${failures} failure(s)` : "\nall passed"); + process.exit(failures ? 1 : 0); +})(); From 4b0f03f53561d04cf5f5322bd5d9c07fa4ba01fa Mon Sep 17 00:00:00 2001 From: luongtrieuvy202 Date: Sat, 26 Sep 2026 21:26:00 +0700 Subject: [PATCH 28/34] fix(channel): let channel.messages list without marking read channel.messages stamps the viewer into _seen_ on every message up to the newest each time it loads. The UI mounts the workspace team chat as a side column on the Files tab, so merely opening a workspace told the team the viewer had read the whole conversation. Optional `mark_read: 0` skips channel_read_messages; the client then marks read through channel.acknowledge when the chat is actually read. Absent = unchanged, so every other caller keeps the old behaviour. Co-Authored-By: Claude Opus 5.5 --- acl/channel.json | 5 +++++ service/private/channel.js | 6 +++++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/acl/channel.json b/acl/channel.json index 36d8726..08934f5 100644 --- a/acl/channel.json +++ b/acl/channel.json @@ -183,6 +183,11 @@ "type": "number", "required": false, "doc": "Page number for pagination (default: 1)" + }, + "mark_read": { + "type": "number", + "required": false, + "doc": "0 = list without marking the messages seen (a workspace team chat shown, not read). Default 1." } }, "returns": { diff --git a/service/private/channel.js b/service/private/channel.js index fafcb8b..1f47984 100644 --- a/service/private/channel.js +++ b/service/private/channel.js @@ -126,6 +126,10 @@ class __private_channel extends Entity { const order = this.input.use(Attr.order, "asc"); const page = this.input.use(Attr.page) || 1; const nid = this.input.use(Attr.nid); + // `mark_read: 0` — the client is showing the history, not reading it (a + // workspace team chat mounted beside the file grid). Absent = the old + // behaviour, so every other caller still marks read on load. + const markRead = `${this.input.use("mark_read", 1)}` !== "0"; let data = await this.db.await_proc( "channel_list_messages", this.uid, @@ -206,7 +210,7 @@ class __private_channel extends Entity { for (const m of messages) { if (!newest || (m.ctime || 0) > (newest.ctime || 0)) newest = m; } - if (newest && newest.message_id) { + if (markRead && newest && newest.message_id) { await this.db.await_proc( "channel_read_messages", newest.message_id, From de190c15f82277aa6bed799cbf7ab45cffdde505 Mon Sep 17 00:00:00 2001 From: EddyOne81 Date: Sat, 26 Sep 2026 08:42:56 -0700 Subject: [PATCH 29/34] fix(activity): per-row read of media and chat notifications persists activity.dismiss_rollup matched the live rollup on its display key_id, but the client sends the key the dismiss acts on: the folder nid for media, the peer's drumate id for chat. Those differ (key_id is the uploader / the contact), so every such read answered INVALID_DATA and the notification came back unread on reload; even a match would have called notification_dismiss with the wrong key. rollupDismissKey() is now the one place that picks that key, shared by mark_all_read (unchanged behaviour) and the per-row read. The live-row check (category, hub, last_id) is kept; forged and stale keys are still refused. Co-Authored-By: Claude Opus 5.5 --- service/private/activity.js | 43 +++++++++++++----- test/activity-mobile-feed-contract.test.js | 53 ++++++++++++++++++++++ 2 files changed, 85 insertions(+), 11 deletions(-) diff --git a/service/private/activity.js b/service/private/activity.js index e49e8f7..67b4ee7 100644 --- a/service/private/activity.js +++ b/service/private/activity.js @@ -453,6 +453,26 @@ function stampBuckets(rows) { return rows; } +// The key notification_dismiss / notification_read act on for a rollup row. +// It is NOT the rollup's display `key_id`: notification_center_next coalesces +// key_id from the contact / drumate first, so a media rollup's key_id is the +// UPLOADER and a p2p chat's is the CONTACT id — while the procs need the folder +// nid (media) and the peer's drumate id (chat). A media rollup whose folder no +// longer resolves has nid NULL and falls back to hub_id, which +// notification_dismiss treats as "the files with no resolvable folder". +// Shared by mark_all_read and the per-row read so both clear the same thing. +function rollupDismissKey(r) { + if (!r) return null; + switch (r.category) { + case 'chat': return r.drumate_id || r.key_id || null; + case 'media': return r.nid || r.hub_id || r.key_id || null; + case 'teamchat': return r.key_id || r.nid || r.hub_id || null; + case 'contact': return r.contact_id || r.key_id || null; + case 'ticket': return r.key_id || r.hub_id || null; + default: return null; + } +} + // --------------------------------------------------------------------------- // Scheduled meetings arrive on TWO channels, and exactly one row must survive. // @@ -777,15 +797,8 @@ class MfsActivity extends Entity { // exactly the rows that tab shows — a teamchat rollup carrying a // meeting_action is cleared by Meeting, not by Chat. if (bucket && bucketOf(r) !== bucket) continue; - let keyId; - switch (r.category) { - case 'chat': keyId = r.drumate_id || r.key_id; break; - case 'media': keyId = r.nid || r.hub_id || r.key_id; break; - case 'teamchat': keyId = r.key_id || r.nid || r.hub_id; break; - case 'contact': keyId = r.contact_id || r.key_id; break; - case 'ticket': keyId = r.key_id || r.hub_id; break; - default: continue; // only rollup categories - } + // null for anything that is not a rollup category, as before. + const keyId = rollupDismissKey(r); if (!keyId) continue; const category = String(r.category); const args = [ @@ -2079,10 +2092,12 @@ class MfsActivity extends Entity { } const row = await this._visibleNotificationRollup(category, keyId, hubId, lastId); if (!row) return this.exception.bad_request('INVALID_DATA'); + // The same key mark_all_read uses. row.key_id is the uploader (media) or + // the contact (chat), which the proc cannot match — the read was a no-op. const result = await this._callUserProc( 'notification_dismiss', category, - String(row.key_id), + String(rollupDismissKey(row) || row.key_id), String(row.hub_id || ''), Number(row.last_id || 0), ); @@ -2379,10 +2394,16 @@ class MfsActivity extends Entity { async _visibleNotificationRollup(category, keyId, hubId, lastId) { if (!ROLLUP_MUTATION_CATEGORIES.has(category)) return null; const rows = await this._notificationRollups(); + // The client sends the key it can act on — the folder nid for media, the + // peer's drumate id for chat (see rollupDismissKey) — which is not the + // rollup's display key_id for those two categories, so matching key_id + // alone rejected every media/chat read with INVALID_DATA. Either key is + // accepted; both are still checked against a row that is live right now. return rows.find(row => ( row && String(row.category || '') === category - && String(row.key_id || '') === keyId + && (String(row.key_id || '') === keyId + || String(rollupDismissKey(row) || '') === keyId) && String(row.hub_id || '') === hubId && (category === 'contact' || Number(row.last_id || 0) === lastId) )) || null; diff --git a/test/activity-mobile-feed-contract.test.js b/test/activity-mobile-feed-contract.test.js index 2fcfbe0..230859b 100644 --- a/test/activity-mobile-feed-contract.test.js +++ b/test/activity-mobile-feed-contract.test.js @@ -172,6 +172,59 @@ test('legacy contact mutation remains canonical without a synthetic last id', as assert.equal(visible.key_id, '0123456789abcdef'); }); +test('media and chat reads resolve by the key the dismiss proc acts on', async () => { + const Activity = require('../service/private/activity'); + const calls = []; + const rollups = [{ + category: 'media', + key_id: 'e5ece49ee5ece4a1', // the uploader's contact, NOT the folder + nid: '8e623a8c8e623a8f', + hub_id: '8e34ce938e34ce95', + last_id: 1399, + }, { + category: 'media', + key_id: 'c4871e5fc4871e6d', + nid: null, // folder no longer resolves + hub_id: 'c4871e5fc4871e6d', + last_id: 142, + }, { + category: 'chat', + key_id: 'e5ece49ee5ece4a1', // the contact id + drumate_id: 'b6e6cdd0b6e6cdd6', + hub_id: 'current-user', + last_id: 1790318086, + }]; + const run = async (params) => { + const context = Object.create(Activity.prototype); + context._notificationRollups = async () => rollups; + context.input = { + need: (k) => params[k], + use: (k) => params[k], + }; + context.exception = {bad_request: code => ({error: code})}; + context.output = {data: d => d}; + context._callUserProc = async (...args) => { calls.push(args); return [{status: 'ok'}]; }; + return context.notification_dismiss(); + }; + + await run({category: 'media', key_id: '8e623a8c8e623a8f', hub_id: '8e34ce938e34ce95', last_id: 1399}); + await run({category: 'media', key_id: 'c4871e5fc4871e6d', hub_id: 'c4871e5fc4871e6d', last_id: 142}); + await run({category: 'chat', key_id: 'b6e6cdd0b6e6cdd6', hub_id: 'current-user', last_id: 1790318086}); + assert.deepEqual(calls, [ + ['notification_dismiss', 'media', '8e623a8c8e623a8f', '8e34ce938e34ce95', 1399], + ['notification_dismiss', 'media', 'c4871e5fc4871e6d', 'c4871e5fc4871e6d', 142], + ['notification_dismiss', 'chat', 'b6e6cdd0b6e6cdd6', 'current-user', 1790318086], + ]); + + // Still only a live row: a stale last_id or an unknown key is refused. + calls.length = 0; + const stale = await run({category: 'media', key_id: '8e623a8c8e623a8f', hub_id: '8e34ce938e34ce95', last_id: 1398}); + const forged = await run({category: 'media', key_id: 'ffffffffffffffff', hub_id: '8e34ce938e34ce95', last_id: 1399}); + assert.deepEqual(stale, {error: 'INVALID_DATA'}); + assert.deepEqual(forged, {error: 'INVALID_DATA'}); + assert.equal(calls.length, 0); +}); + test('support-ticket rollups expose a positive snapshot id', () => { const source = fs.readFileSync( path.join(repositoryRoot, '..', 'schemas', 'drumate', 'procedures', From 65c2d0760b945f3a1d28c988e5f359ed61bbeb3b Mon Sep 17 00:00:00 2001 From: EddyOne81 Date: Sat, 26 Sep 2026 09:29:48 -0700 Subject: [PATCH 30/34] fix(activity): one definition of an unread contact notification Unread OFF marked every undismissed contact_activity row unread, but the tab badges and Unread ON only count events that have an unread source. invite_sent (a second row for the same invitation), invite_received once no longer pending and superseded workspace invites looked unread with no badge counting them (All 14 over 19 unread-looking rows, Other 0 over 5). - CONTACT_UNREAD_PROCS: the one list unread_counts, Unread ON and the new Unread OFF read state are built from; adds contact_invite_accepted_unread so "accepted your invitation" counts. - _alignContactReadState (Unread OFF): a contact row stays unread only if it is counted: an *_unread row, a live workspace-invite or refused rollup, or the newest invite_received of a pending invitation. Only unread -> read, fails open, no cost without unread contact rows; page 1 reuses the rollups already fetched. - Mark as all read on Other now clears everything Other counts: storage alerts, reward expiry, accepted invitations, workspace invites (per hub) and refused invitations. Co-Authored-By: Claude Opus 5.5 --- service/private/activity.js | 175 ++++++++++++++++++--- test/activity-mobile-feed-contract.test.js | 121 ++++++++++++++ 2 files changed, 271 insertions(+), 25 deletions(-) diff --git a/service/private/activity.js b/service/private/activity.js index 67b4ee7..38a5f74 100644 --- a/service/private/activity.js +++ b/service/private/activity.js @@ -629,6 +629,25 @@ function countMeetingsInWindow(rows, start, end) { return n; } +// Every yp.contact_activity event that has its own *_unread procedure. This is +// the ONE list the tab badges (unread_counts), the Unread ON feed and the +// Unread OFF read state (_alignContactReadState) are built from, so the three +// cannot disagree about which contact rows are unread. A new contact_activity +// event that should notify needs its *_unread proc added HERE; one that is not +// listed is shown as read rather than as an unread row nothing counts. +const CONTACT_UNREAD_PROCS = [ + 'contact_task_assigned_unread', + 'contact_task_mention_unread', + 'contact_task_column_change_unread', + 'contact_storage_alert_unread', + // Claim-reward term ending (offline/workers/rewardExpiryWorker.js). + 'contact_reward_expiry_unread', + // Scheduled-meeting notices (room.book/update/remove) → Meeting. + 'contact_meeting_notice_unread', + // " accepted your invitation" → Other. + 'contact_invite_accepted_unread', +]; + const MISSING_PROCS = new Map(); // proc name -> epoch ms to retry after const PROC_RETRY_MS = 60 * 1000; @@ -868,6 +887,13 @@ class MfsActivity extends Entity { [BUCKET.meeting]: [ 'contact_meeting_notice_unread', ], + // The Other tab counts these too (CONTACT_UNREAD_PROCS), so clearing Other + // must clear them, or its badge could never reach zero. + [BUCKET.other]: [ + 'contact_storage_alert_unread', + 'contact_reward_expiry_unread', + 'contact_invite_accepted_unread', + ], }; if (bucket && lookup(CONTACT_CLEAR, bucket)) { for (const proc of lookup(CONTACT_CLEAR, bucket)) { @@ -891,6 +917,43 @@ class MfsActivity extends Entity { } } + // Workspace invitations and refused contact invitations are Other rows too, + // but they come from their own rollup procs, not a *_unread one. Both procs + // return only the NEWEST undismissed row per group, so dismissing that row + // alone would just surface the next older one: + // - hub invites are cleared per workspace, which dismisses every row of + // the group at once (and leaves Accept/Decline alone — those read the + // invitation token, not dismissed_at; see lib/hub-invite-status.js); + // - refused rows are dismissed and re-read until none is left, bounded. + if (bucket === BUCKET.other) { + try { + const invites = toArray(await this._callUserProc('notification_hub_invites')); + const hubs = new Set(); + for (const r of invites) { + const hubId = mapHubInviteRow(r || {}).hub_id; + if (hubId) hubs.add(hubId); + } + for (const hubId of hubs) { + await this.yp.await_proc('contact_activity_dismiss_hub_invite', this.uid, hubId); + } + } catch (e) { + this.warn('[MFS_ACTIVITY] mark_all_read: hub invites not cleared', e && e.message); + } + try { + for (let pass = 0; pass < 5; pass++) { + const refused = toArray(await this._callUserProc('notification_contact_refused')) + .map((r) => parseInt(r && r.id)) + .filter(Boolean); + if (!refused.length) break; + for (const activityId of refused) { + await this._callUserProc('contact_activity_dismiss', this.uid, activityId); + } + } + } catch (e) { + this.warn('[MFS_ACTIVITY] mark_all_read: refused invitations not cleared', e && e.message); + } + } + if (data && data.status === 'ok') { return this.output.data({ status: 'ok', @@ -1039,6 +1102,9 @@ class MfsActivity extends Entity { // these via activity.list / channel.list_notifications for the unread BADGE — // this only changes WHERE they render. Best-effort: a failure never breaks // the rest of the feed. + // Kept for _alignContactReadState below, so it can reuse this call's + // rollups instead of running notification_center_next a second time. + let liveRollups = null; if (filter !== 'mentions' && filter !== 'shares' && page <= 1) { try { // chat/media/teamchat/ticket rollups are NOT returned by @@ -1048,6 +1114,7 @@ class MfsActivity extends Entity { // Unread ON — otherwise the same event double-shows under Unread OFF. const ALWAYS = ROLLUP_CATEGORIES; const rollups = await this._notificationRollups(); + liveRollups = rollups; for (const r of rollups) { if (!r) continue; // Shared-workspace membership is not available to every legacy MFS @@ -1110,20 +1177,7 @@ class MfsActivity extends Entity { // independently best-effort so a missing/failing one never sinks the // others. if (unreadOnly) { - for (const proc of [ - 'contact_task_assigned_unread', - 'contact_task_mention_unread', - 'contact_task_column_change_unread', - 'contact_storage_alert_unread', - // Claim-reward term ending (offline/workers/rewardExpiryWorker.js). - // Added with the event, not after it, per the note above. - 'contact_reward_expiry_unread', - // Scheduled-meeting notices (room.book/update/remove). Under Unread - // OFF these already arrive via activity_get_feed_all's generic - // contact branch, so the feature degrades to "visible with the - // toggle off" if this proc has not been applied yet. - 'contact_meeting_notice_unread', - ]) { + for (const proc of CONTACT_UNREAD_PROCS) { try { const rows = await this._optionalYpProc(proc, this.uid); for (const r of rows) { @@ -1183,6 +1237,7 @@ class MfsActivity extends Entity { await this._stampInviteStatus(result); await this._stampChatMentions(result); result = await this._stampMeetingRollups(result); + if (!unreadOnly) await this._alignContactReadState(result, liveRollups); stampBuckets(result); if (bucket) { @@ -1199,6 +1254,85 @@ class MfsActivity extends Entity { this.output.list(await this._decorateBookmarks(result)); } + /** + * Unread OFF: a contact_activity row reads as unread ONLY if the badge counts + * it, i.e. if one of the sources behind unread_counts / Unread ON returns it. + * + * activity_get_feed_all marks every undismissed contact_activity row unread + * (is_read = dismissed_at IS NULL), but only some events have an unread + * source. The rest — invite_sent (a second row logged for the same + * invitation), invite_received once the invitation is no longer pending, + * older workspace invites superseded by a newer one — showed as unread rows + * that no badge counted and Unread ON never listed (Duy 2026-09-26: All + * showed 14 over 19 unread-looking rows, Other 0 over 5). + * + * The unread set is exactly what is counted: + * - the CONTACT_UNREAD_PROCS rows, + * - the workspace-invite and refused-invitation rollups, + * - for a PENDING contact invitation (counted through the `contact` + * rollup), the newest invite_received from that inviter. + * + * Only ever turns unread into read, never the reverse, and only on rows it + * can match. Fails open: if any source cannot be read, the rows are left as + * the procedure returned them rather than hiding something real. Costs + * nothing unless the page actually holds an unread contact row. + */ + async _alignContactReadState(rows, rollups) { + if (!Array.isArray(rows)) return; + const candidates = rows.filter((r) => ( + r && r.feed_page_source === 'base' && r.event_type === 'contact' + && Number(r.is_read) === 0 && r.id != null + )); + if (!candidates.length) return; + + const unread = new Set(); + let pendingInviters; + try { + const [procRows, hubInvites, refused, pending] = await Promise.all([ + Promise.all(CONTACT_UNREAD_PROCS.map((proc) => this._optionalYpProc(proc, this.uid))), + this._callUserProc('notification_hub_invites'), + this._callUserProc('notification_contact_refused'), + // Page 1 already has the rollups, which respect a dismissed contact + // invitation exactly as the badge does. Later pages use the light + // pending-invitation list instead of recomputing every rollup. + rollups ? null : this._callUserProc('contact_notification_get'), + ]); + // await_proc answers undefined on a SQL error instead of throwing. + if (hubInvites === undefined || refused === undefined || pending === undefined) return; + for (const list of procRows) { + for (const r of list) if (r && r.id != null) unread.add(String(r.id)); + } + for (const r of toArray(hubInvites)) if (r && r.id != null) unread.add(String(r.id)); + for (const r of toArray(refused)) if (r && r.id != null) unread.add(String(r.id)); + pendingInviters = new Set( + (rollups ? rollups.filter((r) => r && r.category === 'contact') : toArray(pending)) + .map((r) => r && r.drumate_id) + .filter(Boolean) + .map(String) + ); + } catch (e) { + this.debug('[ACTIVITY] contact read state left as is', e && e.message); + return; + } + + // The newest undismissed invite_received per pending inviter, as + // contact.invite_get pairs them. + const newestInvite = new Map(); + for (const r of candidates) { + if (r.event !== 'invite_received' || !pendingInviters.has(String(r.uid))) continue; + const cur = newestInvite.get(String(r.uid)); + const newer = !cur + || Number(r.timestamp) > Number(cur.timestamp) + || (Number(r.timestamp) === Number(cur.timestamp) && Number(r.id) > Number(cur.id)); + if (newer) newestInvite.set(String(r.uid), r); + } + for (const r of newestInvite.values()) unread.add(String(r.id)); + + for (const r of candidates) { + if (!unread.has(String(r.id))) r.is_read = 1; + } + } + async _decorateBookmarks(rows) { let saved = []; try { @@ -2299,17 +2433,8 @@ class MfsActivity extends Entity { // otherwise the Meeting badge would read one higher than the rows the tab // actually shows. Collected here, counted below. const contactRows = []; - for (const proc of [ - 'contact_task_assigned_unread', - 'contact_task_mention_unread', - 'contact_task_column_change_unread', - 'contact_storage_alert_unread', - 'contact_reward_expiry_unread', - // Scheduled-meeting notices → Meeting, via BUCKET_BY_EVENT. Listed here - // for the same reason as the rest: the tab badge and the feed must agree - // on what exists. - 'contact_meeting_notice_unread', - ]) { + // The same list the feed uses, so the badge and the rows agree. + for (const proc of CONTACT_UNREAD_PROCS) { try { for (const r of await this._optionalYpProc(proc, this.uid)) if (r) contactRows.push(r); } catch (e) { diff --git a/test/activity-mobile-feed-contract.test.js b/test/activity-mobile-feed-contract.test.js index 230859b..16bf4d4 100644 --- a/test/activity-mobile-feed-contract.test.js +++ b/test/activity-mobile-feed-contract.test.js @@ -274,3 +274,124 @@ test('mark-all fails closed before the global pointer after a rollup failure', a assert.deepEqual(calls, ['notification_center_next', 'notification_read']); assert.ok(!calls.includes('mfs_mark_all_read')); }); + +test('Unread OFF shows a contact row as unread only when the badge counts it', async () => { + const Activity = require('../service/private/activity'); + const calls = []; + const activity = Object.create(Activity.prototype); + activity.uid = 'me'; + activity.debug = () => undefined; + activity._optionalYpProc = async (proc) => ( + proc === 'contact_invite_accepted_unread' ? [{id: 726}] : [] + ); + activity._callUserProc = async (proc) => { + calls.push(proc); + if (proc === 'notification_hub_invites') return [{id: 784}]; + if (proc === 'notification_contact_refused') return []; + throw new Error(`unexpected procedure: ${proc}`); + }; + const base = {feed_page_source: 'base', event_type: 'contact', is_read: 0}; + const rows = [ + {...base, id: 726, event: 'invite_accepted', uid: 'A', timestamp: 90}, + {...base, id: 688, event: 'invite_sent', uid: 'B', timestamp: 100}, + {...base, id: 682, event: 'invite_received', uid: 'B', timestamp: 100}, + {...base, id: 600, event: 'invite_received', uid: 'B', timestamp: 50}, + {...base, id: 700, event: 'invite_received', uid: 'C', timestamp: 70}, + {...base, id: 784, event: 'hub_invite_received', uid: 'D', timestamp: 60}, + {...base, id: 780, event: 'hub_invite_received', uid: 'D', timestamp: 40}, + {...base, id: 9, event: 'task_mention', uid: 'E', is_read: 1}, + {feed_page_source: 'base', event_type: 'mfs', id: 5, event: 'media.new', is_read: 0}, + ]; + + await activity._alignContactReadState(rows, [{category: 'contact', drumate_id: 'B'}]); + + const state = Object.fromEntries(rows.map((r) => [r.id, r.is_read])); + assert.deepEqual(state, { + 726: 0, // counted (contact_invite_accepted_unread) + 688: 1, // duplicate of the invitation + 682: 0, // newest invitation from a pending inviter + 600: 1, // older copy + 700: 1, // inviter no longer pending + 784: 0, // live workspace invite + 780: 1, // superseded workspace invite + 9: 1, + 5: 0, // mfs rows are not touched + }); + // Page 1 reuses the rollups: no pending-invitation lookup. + assert.ok(!calls.includes('contact_notification_get')); +}); + +test('Unread OFF contact read state fails open and uses the light list past page 1', async () => { + const Activity = require('../service/private/activity'); + const make = (answers) => { + const activity = Object.create(Activity.prototype); + activity.uid = 'me'; + activity.debug = () => undefined; + activity._optionalYpProc = async () => []; + activity._callUserProc = async (proc) => answers[proc]; + return activity; + }; + const row = () => [{feed_page_source: 'base', event_type: 'contact', is_read: 0, + id: 682, event: 'invite_received', uid: 'B', timestamp: 1}]; + + // A source that could not be read leaves the rows exactly as they were. + const failed = row(); + await make({notification_hub_invites: undefined, notification_contact_refused: []}) + ._alignContactReadState(failed, [{category: 'contact', drumate_id: 'B'}]); + assert.equal(failed[0].is_read, 0); + + // Page 2+: pending inviters come from contact_notification_get. + const pending = row(); + await make({notification_hub_invites: [], notification_contact_refused: [], + contact_notification_get: [{drumate_id: 'B'}]})._alignContactReadState(pending, null); + assert.equal(pending[0].is_read, 0); + const gone = row(); + await make({notification_hub_invites: [], notification_contact_refused: [], + contact_notification_get: []})._alignContactReadState(gone, null); + assert.equal(gone[0].is_read, 1); +}); + +test('Mark as all read on Other clears every Other source it counts', async () => { + const Activity = require('../service/private/activity'); + const user = []; + const yp = []; + let refusedReads = 0; + const context = { + uid: 'me', + input: {get: () => 0, use: (k) => (k === 'bucket' ? 'other' : undefined)}, + debug: () => undefined, + warn: () => undefined, + output: {data: d => d}, + yp: {await_proc: async (...args) => { yp.push(args); return [{status: 'ok'}]; }}, + async _optionalYpProc(proc) { + return proc === 'contact_invite_accepted_unread' ? [{id: 726}] : []; + }, + async _callUserProc(proc, ...args) { + user.push([proc, ...args]); + if (proc === 'notification_center_next') return []; + if (proc === 'notification_hub_invites') { + return [{id: 2, data: '{"hub_id":"h1"}'}, {id: 3, data: '{"hub_id":"h2"}'}]; + } + if (proc === 'notification_contact_refused') { + refusedReads += 1; + return refusedReads === 1 ? [{id: 7}] : []; + } + return [{status: 'ok'}]; + }, + }; + + const result = await Activity.prototype.mark_all_read.call(context); + + assert.equal(result.status, 'ok'); + assert.equal(result.bucket, 'other'); + assert.deepEqual( + user.filter(([p]) => p === 'contact_activity_dismiss').map(([, , id]) => id), + [726, 7], + ); + assert.deepEqual( + yp.filter(([p]) => p === 'contact_activity_dismiss_hub_invite').map(([, , hub]) => hub), + ['h1', 'h2'], + ); + // Other never touches the Files pointers. + assert.ok(!user.some(([p]) => p === 'mfs_mark_all_read')); +}); From 1b1a1564c4b1b6b025d36fb3262ccf01f8af7a62 Mon Sep 17 00:00:00 2001 From: EddyOne81 Date: Sat, 26 Sep 2026 09:52:26 -0700 Subject: [PATCH 31/34] fix(activity): reading a contact notification keeps it in history activity.read_contact_event marks one contact_activity row read via contact_activity_mark_read (dismissed_at only). dismiss_contact_event keeps its removal meaning for mobile. Tab-scoped Mark as all read (Task / Meeting / Other) now marks those rows read too instead of hiding them, as the unscoped one always did. Until the new proc is applied, _markContactRead falls back to contact_activity_dismiss so a read is never lost during a rollout. Co-Authored-By: Claude Opus 5.5 --- acl/activity.json | 32 ++++++++++++++++ service/private/activity.js | 37 +++++++++++++++++- test/activity-mobile-feed-contract.test.js | 44 +++++++++++++++++++++- 3 files changed, 110 insertions(+), 3 deletions(-) diff --git a/acl/activity.json b/acl/activity.json index 279fa18..0f07621 100644 --- a/acl/activity.json +++ b/acl/activity.json @@ -500,6 +500,38 @@ } }, + "read_contact_event": { + "doc": "Mark a single contact_activity row (task, meeting, invitation, etc.) as read for the caller. Stamps yp.contact_activity.dismissed_at only, so the row stays in Activity history with is_read = 1. Removal is dismiss_contact_event / delete_contact_event.", + "scope": "hub", + "permission": { + "src": "read" + }, + "params": { + "activity_id": { + "type": "integer", + "required": true, + "description": "The yp.contact_activity row ID to mark read" + } + }, + "returns": { + "type": "object", + "properties": { + "status": { + "type": "string", + "description": "ok on success" + }, + "activity_id": { + "type": "integer", + "description": "The contact_activity ID" + }, + "dismissed_at": { + "type": "integer", + "description": "Unix timestamp of the read" + } + } + } + }, + "dismiss_contact_event": { "doc": "Dismiss a single contact_activity row (hub invite, contact invite, etc.) so it no longer appears in the activity feed. Stamps yp.contact_activity.dismissed_at without deleting the underlying audit row.", "scope": "hub", diff --git a/service/private/activity.js b/service/private/activity.js index 38a5f74..4341ad5 100644 --- a/service/private/activity.js +++ b/service/private/activity.js @@ -903,7 +903,9 @@ class MfsActivity extends Entity { const activityId = parseInt(r && r.id); if (!activityId) continue; try { - await this._callUserProc('contact_activity_dismiss', this.uid, activityId); + // Read, not removed: the rows stay in Activity history, exactly + // as an unscoped Mark as all read leaves them. + await this._markContactRead(activityId); } catch (e) { this.warn('[MFS_ACTIVITY] mark_all_read: contact dismiss failed', bucket, activityId, e && e.message); } @@ -946,7 +948,7 @@ class MfsActivity extends Entity { .filter(Boolean); if (!refused.length) break; for (const activityId of refused) { - await this._callUserProc('contact_activity_dismiss', this.uid, activityId); + await this._markContactRead(activityId); } } } catch (e) { @@ -2123,6 +2125,37 @@ class MfsActivity extends Entity { this.output.data(data); } + /** + * Mark a single contact_activity row READ, leaving it in Activity history. + * + * dismiss_contact_event also stamps hidden_at (removal — what mobile relies + * on), so the web panel recording a read through it made the notification + * vanish from the Unread OFF list. This only writes dismissed_at, the read + * marker activity_get_feed_all turns into is_read = 1. + * Endpoint: POST /activity.read_contact_event + * Input: activity_id (integer) + */ + async read_contact_event() { + const activityId = parseInt(this.input.need('activity_id')); + if (!Number.isSafeInteger(activityId) || activityId < 1) { + return this.exception.bad_request('INVALID_DATA'); + } + const rows = await this._markContactRead(activityId); + this.output.data(toArray(rows)[0] || {}); + } + + /** + * Record one contact_activity row as read (dismissed_at only). Until + * contact_activity_mark_read is applied to a database, falls back to + * contact_activity_dismiss — read + hidden, the previous behaviour — so a + * read is never silently lost during a rollout. + */ + async _markContactRead(activityId) { + const { ok, rows } = await this._optionalYpProcResult('contact_activity_mark_read', this.uid, activityId); + if (ok) return rows; + return this._callUserProc('contact_activity_dismiss', this.uid, activityId); + } + /** * Hide a single contact_activity row (hub invite, contact invite, etc.) * from the user's activity feed. Underlying event stays around for audit. diff --git a/test/activity-mobile-feed-contract.test.js b/test/activity-mobile-feed-contract.test.js index 16bf4d4..553d77b 100644 --- a/test/activity-mobile-feed-contract.test.js +++ b/test/activity-mobile-feed-contract.test.js @@ -366,6 +366,11 @@ test('Mark as all read on Other clears every Other source it counts', async () = async _optionalYpProc(proc) { return proc === 'contact_invite_accepted_unread' ? [{id: 726}] : []; }, + async _optionalYpProcResult(proc, ...args) { + yp.push([proc, ...args]); + return {ok: true, rows: [{status: 'ok'}]}; + }, + _markContactRead: Activity.prototype._markContactRead, async _callUserProc(proc, ...args) { user.push([proc, ...args]); if (proc === 'notification_center_next') return []; @@ -384,10 +389,12 @@ test('Mark as all read on Other clears every Other source it counts', async () = assert.equal(result.status, 'ok'); assert.equal(result.bucket, 'other'); + // Marked READ (dismissed_at only), never removed from history. assert.deepEqual( - user.filter(([p]) => p === 'contact_activity_dismiss').map(([, , id]) => id), + yp.filter(([p]) => p === 'contact_activity_mark_read').map(([, , id]) => id), [726, 7], ); + assert.ok(!user.some(([p]) => p === 'contact_activity_dismiss')); assert.deepEqual( yp.filter(([p]) => p === 'contact_activity_dismiss_hub_invite').map(([, , hub]) => hub), ['h1', 'h2'], @@ -395,3 +402,38 @@ test('Mark as all read on Other clears every Other source it counts', async () = // Other never touches the Files pointers. assert.ok(!user.some(([p]) => p === 'mfs_mark_all_read')); }); + +test('reading a contact notification marks it read without removing it', async () => { + const Activity = require('../service/private/activity'); + const make = (applied) => { + const calls = []; + const activity = Object.create(Activity.prototype); + activity.uid = 'me'; + activity.input = {need: () => '726'}; + activity.exception = {bad_request: code => ({error: code})}; + activity.output = {data: d => { activity.sent = d; return d; }}; + activity._optionalYpProcResult = async (proc, ...args) => { + calls.push([proc, ...args]); + return applied ? {ok: true, rows: [{status: 'ok', activity_id: 726}]} : {ok: false, rows: []}; + }; + activity._callUserProc = async (proc, ...args) => { + calls.push([proc, ...args]); + return [{status: 'ok', activity_id: 726}]; + }; + return {activity, calls}; + }; + + const live = make(true); + await live.activity.read_contact_event(); + assert.deepEqual(live.activity.sent, {status: 'ok', activity_id: 726}); + assert.deepEqual(live.calls, [['contact_activity_mark_read', 'me', 726]]); + + // Proc not applied yet: fall back to the previous read (+hide), never lose the read. + const rollout = make(false); + await rollout.activity.read_contact_event(); + assert.deepEqual(rollout.calls.map(([p]) => p), ['contact_activity_mark_read', 'contact_activity_dismiss']); + + const bad = make(true); + bad.activity.input = {need: () => 'x'}; + assert.deepEqual(await bad.activity.read_contact_event(), {error: 'INVALID_DATA'}); +}); From e062cb08581a0cc35c887a88135b10a249e07fb5 Mon Sep 17 00:00:00 2001 From: EddyOne81 Date: Sat, 26 Sep 2026 22:35:21 -0700 Subject: [PATCH 32/34] feat(activity): pin bookmarked notifications on top (bookmark_rows) - bookmark_add accepts an optional `row`; it is kept as a snapshot only when bookmarkKey(row) reproduces the key and it is under 16 KB. Without it (mobile) nothing changes. - bookmark_remove also removes the snapshot. - New activity.bookmark_rows: the saved rows, newest first, optional tab bucket, deleted rows dropped, served is_read=1 (the client prefers the live feed row, which carries the true state). get_feed is untouched. Co-Authored-By: Claude Opus 5.5 --- acl/activity.json | 15 +++++- service/private/activity.js | 99 ++++++++++++++++++++++++++++++++++++- 2 files changed, 111 insertions(+), 3 deletions(-) diff --git a/acl/activity.json b/acl/activity.json index 0f07621..5dff2a1 100644 --- a/acl/activity.json +++ b/acl/activity.json @@ -631,11 +631,12 @@ }, "bookmark_add": { - "doc": "Save one visible Activity item in the caller's personal notification bookmark store. The key is a server-generated SHA-256 activity identity.", + "doc": "Save one visible Activity item in the caller's personal notification bookmark store. The key is a server-generated SHA-256 activity identity. The web panel may also send the row itself, kept (when it matches the key) so bookmark_rows can pin it on top.", "scope": "hub", "permission": { "src": "read" }, "params": { - "bookmark_key": { "type": "string", "required": true } + "bookmark_key": { "type": "string", "required": true }, + "row": { "type": "object", "required": false } }, "returns": { "type": "object", @@ -662,6 +663,16 @@ } }, + "bookmark_rows": { + "doc": "The caller's saved Activity items (snapshots stored by bookmark_add), newest first, to pin on top of the panel. Optional bucket scopes to one tab. is_read is always 1: the client uses the live feed row when it has one.", + "scope": "hub", + "permission": { "src": "read" }, + "params": { + "bucket": { "type": "string", "required": false } + }, + "returns": { "type": "array" } + }, + "read": { "doc": "Mark a rollup as read (advance read pointer) without hiding it. For chat/teamchat/ticket this is identical to dismiss; for media we only advance mfs_ack; for contact this is a no-op (use dismiss to hide).", "scope": "hub", diff --git a/service/private/activity.js b/service/private/activity.js index 4341ad5..7f2019f 100644 --- a/service/private/activity.js +++ b/service/private/activity.js @@ -171,6 +171,13 @@ function bookmarkKey(row) { return createHash('sha256').update(JSON.stringify(identity)).digest('hex'); } +// A saved row's snapshot (bookmark_rows) never keeps per-render or per-state +// fields: they are recomputed when the row is served back. +const BOOKMARK_ROW_DROP = ['is_saved', 'is_read', 'day_header', 'pinned_source', 'feed_page_source']; +// Plenty for any feed row (the largest carry a few hundred bytes of JSON), and +// small enough that 1000 bookmarks cannot grow a user's database by much. +const BOOKMARK_ROW_MAX = 16 * 1024; + // Surface task fields at the top level from the nested contact_activity `data` // JSON so the client renders the right text and can navigate to the task, // without relying on the nested JSON surviving the LETC model. Handles BOTH @@ -2610,7 +2617,15 @@ class MfsActivity extends Entity { 'notification_activity_bookmark_add', bookmarkKey, ); - return this.output.data(toArray(result)[0] || {}); + const data = toArray(result)[0] || {}; + // Optional: the web panel also sends the row itself so a saved row can be + // pinned on top whatever feed page it sits on (bookmark_rows). Mobile sends + // only the key and is unaffected. Best-effort: the bookmark above is the + // source of truth and stands even if the snapshot is refused or fails. + if (Number(data.is_saved) === 1) { + await this._saveBookmarkRow(bookmarkKey, this.input.use('row')); + } + return this.output.data(data); } async bookmark_remove() { @@ -2622,9 +2637,91 @@ class MfsActivity extends Entity { 'notification_activity_bookmark_remove', bookmarkKey, ); + try { + await this._callUserProc('notification_activity_bookmark_row_remove', bookmarkKey); + } catch (e) { + this.debug('[ACTIVITY] bookmark row remove skipped', e && e.message); + } return this.output.data(toArray(result)[0] || {}); } + /** + * Store what a saved row looked like, keyed by its bookmark. + * + * The row comes from the client, so it is only kept when it IS the row the + * key names -- bookmarkKey(row) must reproduce the key -- and when it is a + * reasonable size. It is only ever returned to the same user (bookmark_rows), + * whose panel renders it through the same escaping as any feed row. + */ + async _saveBookmarkRow(key, row) { + if (!row || typeof row !== 'object' || Array.isArray(row)) return; + try { + if (bookmarkKey(row) !== key) { + this.debug('[ACTIVITY] bookmark row does not match its key, not stored'); + return; + } + const clean = { ...row }; + for (const k of BOOKMARK_ROW_DROP) delete clean[k]; + const payload = JSON.stringify(clean); + if (payload.length > BOOKMARK_ROW_MAX) { + this.debug('[ACTIVITY] bookmark row too large, not stored', payload.length); + return; + } + stampBuckets([clean]); + const rowTime = Math.max(0, parseInt(clean.timestamp || clean.ctime, 10) || 0); + await this._callUserProc( + 'notification_activity_bookmark_row_save', + key, + validBucket(clean.bucket) || '', + rowTime, + payload, + ); + } catch (e) { + this.debug('[ACTIVITY] bookmark row save skipped', e && e.message); + } + } + + /** + * The rows the user saved, newest notification first, to pin on top of the + * panel. Scoped to a tab when `bucket` names one. Rows the user has since + * deleted with the trash button are dropped, like get_feed does. + * + * is_read is forced to 1: the snapshot's read state is as old as the save and + * cannot be trusted, and the client replaces a snapshot with the live row + * whenever the feed has it, which carries the true state. + * Endpoint: POST /activity.bookmark_rows + */ + async bookmark_rows() { + const bucket = validBucket(this.input.use('bucket')); + let rows = []; + try { + rows = toArray(await this._callUserProc( + 'notification_activity_bookmark_row_list', + bucket || '', + )); + } catch (e) { + this.debug('[ACTIVITY] bookmark rows unavailable', e && e.message); + return this.output.list([]); + } + let result = []; + for (const r of rows) { + if (!r || !r.payload) continue; + let row; + try { row = JSON.parse(r.payload); } catch (e) { continue; } + if (!row || typeof row !== 'object') continue; + result.push({ + ...row, + bucket: row.bucket || r.bucket || undefined, + bookmark_key: r.bookmark_key, + is_saved: 1, + is_read: 1, + pinned_source: 'snapshot', + }); + } + result = await this._dropDeleted(result); + return this.output.list(result); + } + /** * Mark a rollup as read without hiding it. For backends that distinguish * between read-pointer and dismissed-flag (contact, mfs_changelog), we only From 2f773d361b649dc68522d89ecdfbf271520ab0f7 Mon Sep 17 00:00:00 2001 From: Tran Hoang Huan <121786621+tranh0anghuan@users.noreply.github.com> Date: Sun, 27 Sep 2026 17:24:34 +0700 Subject: [PATCH 33/34] Feat/trash filters (#237) * feat(media): optional sort on show_bin (latest/earliest/expiring) Co-Authored-By: Claude Opus 5.5 (1M context) * fix(trash): name the sorted bin proc mfs_show_bin_sorted mfs_show_bin_next already exists as a one-arg proc (factory templates, 1668 stage instances). Reusing the name with two args would change its arity on a shared DB, and new instances built from the templates would answer an argument-count error. Co-Authored-By: Claude Opus 5.5 (1M context) --------- Co-authored-by: Drumee Dev Co-authored-by: Claude Opus 5.5 (1M context) --- acl/media.json | 7 +++++++ service/lib/trash-sort.js | 21 +++++++++++++++++++++ service/private/media.js | 4 +++- test/trash-sort.test.js | 30 ++++++++++++++++++++++++++++++ 4 files changed, 61 insertions(+), 1 deletion(-) create mode 100644 service/lib/trash-sort.js create mode 100644 test/trash-sort.test.js diff --git a/acl/media.json b/acl/media.json index 9aa7a86..d348f24 100644 --- a/acl/media.json +++ b/acl/media.json @@ -855,6 +855,13 @@ "default": 1, "min": 1, "doc": "Page number for pagination" + }, + "sort": { + "type": "string", + "required": false, + "default": "latest", + "enum": ["latest", "earliest", "expiring"], + "doc": "latest: most recently deleted first. earliest: oldest deletion first. expiring: only items with 5 days or less left before auto-purge, fewest days first." } }, "returns": { diff --git a/service/lib/trash-sort.js b/service/lib/trash-sort.js new file mode 100644 index 0000000..43b98bd --- /dev/null +++ b/service/lib/trash-sort.js @@ -0,0 +1,21 @@ +/** + * media.show_bin `sort` values. Mirrors acl/media.json's enum and + * mfs_show_bin_sorted's accepted _sort values. + * + * `latest` (the default, and the only order before the param existed) keeps + * calling the one-arg mfs_show_bin: that name exists on every instance, + * including any the mfs_show_bin_sorted patch has not reached yet. + */ +const SORTS = Object.freeze(['latest', 'earliest', 'expiring']); +const DEFAULT_SORT = 'latest'; + +function trashSort(value) { + return SORTS.includes(value) ? value : DEFAULT_SORT; +} + +function showBinCall(page, sort) { + const s = trashSort(sort); + return s === DEFAULT_SORT ? ['mfs_show_bin', page] : ['mfs_show_bin_sorted', page, s]; +} + +module.exports = { SORTS, DEFAULT_SORT, trashSort, showBinCall }; diff --git a/service/private/media.js b/service/private/media.js index f713655..4fb7bfa 100644 --- a/service/private/media.js +++ b/service/private/media.js @@ -50,6 +50,7 @@ const Media = require("../media"); const { writeAudit } = require("./_audit"); const { movePlanRows } = require("./_move-plan"); const { createHub } = require("../lib/env"); +const { showBinCall } = require("../lib/trash-sort"); const { ARCHIVE_EXTENSIONS, SMALL_MAX_BYTES, SMALL_MAX_ENTRIES, UNEXTRACTABLE_EXTENSIONS, inspect, @@ -2747,7 +2748,8 @@ class __private_media extends Media { // sent 0 — i.e. the default was unreachable, not merely unused. let page = this.input.get(Attr.page); if (page == null || page == undefined || page == 0) page = 1; - this.db.call_proc("mfs_show_bin", page, this.output.list); + const [proc, ...args] = showBinCall(page, this.input.get('sort')); + this.db.call_proc(proc, ...args, this.output.list); } /** diff --git a/test/trash-sort.test.js b/test/trash-sort.test.js new file mode 100644 index 0000000..d99dd52 --- /dev/null +++ b/test/trash-sort.test.js @@ -0,0 +1,30 @@ +const assert = require('node:assert/strict'); +const test = require('node:test'); + +const { SORTS, DEFAULT_SORT, trashSort, showBinCall } = require('../service/lib/trash-sort'); + +test('accepts the three sorts verbatim', () => { + for (const s of ['latest', 'earliest', 'expiring']) assert.equal(trashSort(s), s); +}); + +test('anything else falls back to latest', () => { + for (const s of [undefined, null, '', 'undefined', 'LATEST', 'oldest', 1, {}]) { + assert.equal(trashSort(s), DEFAULT_SORT); + } +}); + +test('latest keeps calling the one-arg proc, so an unpatched instance still answers', () => { + assert.deepEqual(showBinCall(2, 'latest'), ['mfs_show_bin', 2]); + assert.deepEqual(showBinCall(1, undefined), ['mfs_show_bin', 1]); +}); + +test('earliest / expiring call mfs_show_bin_sorted with the sort', () => { + assert.deepEqual(showBinCall(1, 'earliest'), ['mfs_show_bin_sorted', 1, 'earliest']); + assert.deepEqual(showBinCall(3, 'expiring'), ['mfs_show_bin_sorted', 3, 'expiring']); +}); + +test('acl/media.json show_bin.sort enum mirrors SORTS', () => { + const { sort } = require('../acl/media.json').services.show_bin.params; + assert.deepEqual(sort.enum, SORTS); + assert.equal(sort.default, DEFAULT_SORT); +}); From 1a05c2631e7c8d0b70414db5156d8dd3df60f343 Mon Sep 17 00:00:00 2001 From: EddyOne81 Date: Sun, 27 Sep 2026 21:56:53 -0700 Subject: [PATCH 34/34] =?UTF-8?q?feat(activity):=20unread=5Fcounts.files?= =?UTF-8?q?=5Fby=5Fhub=20=E2=80=94=20new=20files=20and=20folders=20per=20w?= =?UTF-8?q?orkspace?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For the desk rail's Files pill (Duy 2026-09-27): uploads, new files AND new folders of a workspace, counted by the new drumate routine mfs_new_by_hub (schemas 6610321). Optional `files_since` = { "": ts } — the web client's "Files tab last opened over" marks — so only what came after counts. Additive and fail-safe: - a new field on unread_counts, not added to `files` or `all` (those already count every changelog event), so every current number is unchanged and clients that ignore it are unaffected; - files_since is sanitised (16-hex hub ids, positive ints, <= 500 keys) and passed as a bound JSON parameter; - [] while the routine is absent, with the MISSING_PROCS cooldown keyed per user DB so one stale DB never silences another; never throws. No new request: the panel already calls unread_counts on every refresh. Checked with stubs (sanitiser, mapping, cooldown per DB, error path); server suite: the only failures are the pre-existing invite-mail ones (present on clean origin/test too). Co-Authored-By: Claude Opus 5.5 (cherry picked from commit 94f736ed4a836f79f5af7210d6bf97ba2d13d157) --- acl/activity.json | 13 +++++++- service/private/activity.js | 66 ++++++++++++++++++++++++++++++++++++- 2 files changed, 77 insertions(+), 2 deletions(-) diff --git a/acl/activity.json b/acl/activity.json index 5dff2a1..34e4c40 100644 --- a/acl/activity.json +++ b/acl/activity.json @@ -31,7 +31,13 @@ "permission": { "src": "read" }, - "params": {}, + "params": { + "files_since": { + "type": "object", + "required": false, + "description": "Web rail only: { \"\": } - per workspace, count files_by_hub only after the last time the user opened its Files tab. Malformed entries are ignored." + } + }, "returns": { "type": "object", "properties": { @@ -64,6 +70,11 @@ "type": "integer", "description": "Other tab: workspace and team invites, contacts, tickets, access requests and system alerts", "example": 1 + }, + "files_by_hub": { + "type": "array", + "description": "Per workspace, NEW files and folders (media.new; not chat attachments, not workspace creation) newer than files_since: [{ hub_id, cnt, last_ts }]. Not added to any total above. [] until mfs_new_by_hub is deployed.", + "example": [{ "hub_id": "ac248fb1ac248fb9", "cnt": 3, "last_ts": 1790568840 }] } } }, diff --git a/service/private/activity.js b/service/private/activity.js index 7f2019f..ceb43bf 100644 --- a/service/private/activity.js +++ b/service/private/activity.js @@ -658,6 +658,31 @@ const CONTACT_UNREAD_PROCS = [ const MISSING_PROCS = new Map(); // proc name -> epoch ms to retry after const PROC_RETRY_MS = 60 * 1000; +// unread_counts `files_since`: the web client's per-workspace "Files tab last +// opened over" marks, { "": }. Only well-formed entries are +// kept and the map is capped, so a malformed or oversized value can only mean +// "no marks" — it never reaches the procedure as anything but a small JSON +// document, passed as a bound parameter. +const FILES_SINCE_MAX_KEYS = 500; +function filesSinceArg(value) { + let v = value; + if (typeof v === 'string') { + try { v = JSON.parse(v); } catch (e) { v = null; } + } + const out = {}; + if (!v || typeof v !== 'object' || Array.isArray(v)) return '{}'; + let n = 0; + for (const [hub, ts] of Object.entries(v)) { + if (n >= FILES_SINCE_MAX_KEYS) break; + if (!/^[0-9a-f]{16}$/.test(hub)) continue; + const t = parseInt(ts, 10); + if (!Number.isFinite(t) || t <= 0) continue; + out[hub] = t; + n += 1; + } + return JSON.stringify(out); +} + // A caller-supplied bucket is only honoured when it names one of the 5 tabs; // anything else (absent, empty, typo'd) means "no bucket scope" and every path // keeps its pre-existing, unscoped behaviour. @@ -2553,7 +2578,46 @@ class MfsActivity extends Entity { } const all = counts.files + counts.task + counts.meeting + counts.chat + counts.other; - this.output.data({ all, ...counts }); + // 6. Per workspace, the NEW files and folders — the desk rail's Files pill. + // Its own field, NOT part of `files` above (that already counts every + // changelog event; adding these again would double them). Additive: a + // client that does not read it is unaffected, and it is [] while the + // procedure is not deployed. + const files_by_hub = await this._newFilesByHub(filesSinceArg(this.input.use('files_since'))); + this.output.data({ all, ...counts, files_by_hub }); + } + + /** + * mfs_new_by_hub (drumate DB): [{ hub_id, cnt, last_ts }] of media.new + * events — files and folders — per workspace, newer than the caller's marks. + * Best-effort, never throws. A DB that does not have the routine yet (the + * rollout window, or a user DB provisioned before it) is skipped for + * PROC_RETRY_MS, keyed PER DATABASE so one stale DB never silences another. + */ + async _newFilesByHub(since) { + const key = `user-db:${this.user.get(Attr.db_name)}:mfs_new_by_hub`; + const now = Date.now(); + const retryAfter = MISSING_PROCS.get(key); + if (retryAfter && now < retryAfter) return []; + try { + const rows = await this._callUserProc('mfs_new_by_hub', this.uid, since); + if (rows === undefined) { + MISSING_PROCS.set(key, now + PROC_RETRY_MS); + return []; + } + if (retryAfter) MISSING_PROCS.delete(key); + const out = []; + for (const r of toArray(rows)) { + if (!r || !r.hub_id) continue; + const cnt = parseInt(r.cnt, 10) || 0; + if (cnt <= 0) continue; + out.push({ hub_id: String(r.hub_id), cnt, last_ts: parseInt(r.last_ts, 10) || 0 }); + } + return out; + } catch (e) { + this.debug('[ACTIVITY] unread_counts: mfs_new_by_hub skipped', e && e.message); + return []; + } } async _visibleNotificationRollup(category, keyId, hubId, lastId) {