From 00b4483bfd8d3b9e13de0305879f97596fa50a8d Mon Sep 17 00:00:00 2001 From: luongtrieuvy202 Date: Mon, 14 Sep 2026 17:47:47 +0700 Subject: [PATCH 01/15] fix(hub): grant workspace invitations to existing accounts in any domain MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An invitation sent to somebody who already has a Drumee account could end up in yp.pending_invitation instead of being granted, and nothing ever redeems a pending row for an account that already exists — both redeemers ran at account creation only. No add_member means no join_hub, so no workspace under the invitee's home root and nothing for desk.home to list: the workspace was invisible on their desk, reload after reload. hub.add_contributors and hub.invite_with_roles granted membership only when the address was an ACTIVE CONTACT of the caller, or when the account lived in the caller's own domain_id. Both describe the INVITER, not the invitee's ability to hold a membership, and hub.invite (the popup, the folder access panel, the restricted permission panel) has never applied either. Every self-signup sits in domain 1 until org_provision moves it, so any invitation from a provisioned org to a free user landed in the pending table — which is why this never reproduced on test, where no org has been provisioned and everybody shares domain 1. Both endpoints now resolve the entity to an account and grant on the spot, whatever domain it is in. Pending rows are reserved for addresses with no account, which is the only case anything redeemed. Alongside: - service/lib/resolve-pending-invitation.js — the two byte-for-byte copies of the redeemer in signup.js and butler.js become one module, and yp.login runs it after a successful sign-in so an account already carrying an orphaned row repairs itself. It refuses to walk over a membership that already works (a stale row must not demote a privilege raised since, nor re-run join_hub over a workspace the user has had for months), converts the stored absolute expiry back into the hours add_member expects, grants the chat-upload permission _grantMembership makes, writes the bell notification, and keeps rows that failed so a later attempt retries them. - invite_with_roles pushed its member notification with no service name, so options.service arrived undefined and every desk consumer dropped it — a correctly granted member still had to reload. It now sends hub.invite_received like the other two call sites. - service/scripts/redeem-orphan-invitations.js — dry-run by default, repairs the accounts already holding un-granted invitations. - test/pending-invitation-redeem.test.js — 14 tests over the redeemer. Co-Authored-By: Claude Opus 5 (1M context) --- service/butler.js | 113 +----- service/lib/resolve-pending-invitation.js | 360 +++++++++++++++++++ service/private/hub.js | 326 ++++++++--------- service/scripts/redeem-orphan-invitations.js | 133 +++++++ service/signup.js | 94 +---- service/yp.js | 61 ++++ test/pending-invitation-redeem.test.js | 250 +++++++++++++ 7 files changed, 986 insertions(+), 351 deletions(-) create mode 100644 service/lib/resolve-pending-invitation.js create mode 100644 service/scripts/redeem-orphan-invitations.js create mode 100644 test/pending-invitation-redeem.test.js diff --git a/service/butler.js b/service/butler.js index 27df3e01..ccf427b8 100644 --- a/service/butler.js +++ b/service/butler.js @@ -16,13 +16,12 @@ */ const { - Attr, Constants, sendSms, Messenger, Cache, sysEnv, uniqueId, RedisStore + Attr, Constants, sendSms, Messenger, Cache, sysEnv, uniqueId } = require("@drumee/server-essentials"); const { platform } = require('./lib/env'); const { logConnection } = require('./lib/connection_log'); const { resolve } = require('path'); const { isEmpty, isString } = require("lodash"); -const { notifyMemberJoined } = require("./lib/notify-member-joined"); const { PASS_CHECKER, ID_NOBODY, @@ -708,112 +707,16 @@ class __butler extends Mfs { * After a new user completes signup, resolve any pending hub invitations * that were registered via yp_add_pending_invitation before the account existed. * + * The body of this used to live here, duplicated byte-for-byte in signup.js, + * and reachable from account creation ONLY. Both copies now defer to + * `service/lib/resolve-pending-invitation`, which the login path and the + * repair script share — see that module for why that matters. + * * @param {string} email - newly registered user's email */ async _resolve_pending_invitation(email) { - const { toArray } = require("@drumee/server-essentials").utils; - const { isEmpty } = require("lodash"); - - // 1. Get new user's ID - const newUser = await this.yp.await_proc("drumate_exists", email); - if (isEmpty(newUser) || !newUser.id) { - this.warn("[_resolve_pending_invitation] Cannot find newly created user for", email); - return; - } - - // 2. Find all pending hub invitations for this email - const pending = await this.yp.await_proc("pending_invitation_get_by_email", email); - const rows = toArray(pending); - if (isEmpty(rows)) return; - - for (const row of rows) { - const { hub_id, permission, expiry_time } = row; - try { - // 3. Get hub db_name explicitly - const db_name = await this.yp.await_func("get_db_name", hub_id); - if (!db_name) { - this.warn(`[_resolve_pending_invitation] Cannot find db_name for hub ${hub_id}`); - continue; - } - - // 4. Add as real member - await this.yp.await_proc(`${db_name}.add_member`, newUser.id, permission, expiry_time); - - // 5. Grant hub permission - await this.yp.await_proc( - `${db_name}.permission_grant`, - '*', newUser.id, expiry_time, permission, 'system', 'Resolved from pending_invitation on signup' - ); - - // Audit: invite redeemed on signup — never let a failure break the join. - try { - const { writeAudit } = require("./private/_audit"); - await writeAudit(this, { - db: db_name, - uid: newUser.id, - action: 'invite_accepted', - category: 'member', - notify_to: 'admin', - entity_id: hub_id, - log: `Invite accepted — ${email} joined the workspace on signup`, - }); - } catch (e) { /* audit only */ } - - // 6. Notify the (now online) user so the desk sidebar can pick up the - // new workspace, mirroring hub.invite()'s notification for drumates. - try { - const hub = await this.yp.await_proc(`${db_name}.mfs_access_node`, newUser.id, hub_id); - if (hub) { - hub.ownpath = '/'; - hub.hub_id = hub.actual_hub_id; - hub.db_name = hub.actual_db; - const sockets = await this.yp.await_proc('user_sockets', newUser.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(`[_resolve_pending_invitation] WS notify failed for hub ${hub_id}:`, err && err.message); - } - - // Tell online members a member joined so an open permission matrix - // refreshes. (butler.js has no direct service boundary with hub.js, so - // the shared helper is used instead of an inherited method.) - await notifyMemberJoined(this, hub_id, newUser.id); - - this.debug(`[_resolve_pending_invitation] Added user ${newUser.id} to hub ${hub_id}`); - } catch (err) { - // Log per-hub failure but continue processing remaining hubs - this.warn(`[_resolve_pending_invitation] Failed for hub ${hub_id}:`, err && err.message); - } - } - - // 6. Clean up all resolved pending_invitation entries for this email - - // Viral loop: every pending invitation for this address is redeemed by this - // one sign-up, so the acceptance is recorded once for all of them rather - // than per hub — invite_track_accept stamps every row still open for the - // address. Placed BEFORE pending_invitation_delete_by_email below, which - // erases the only other evidence the invitations ever existed. - // - // Never allowed to throw: an account that was created and joined its - // workspaces, then reported a failure because a metric could not be - // written, is strictly worse than an uncounted acceptance. - try { - await this.yp.await_proc("invite_track_accept", email, newUser.id); - } catch (err) { - this.warn("[butler._resolve_pending_invitation] invite tracking failed for", email, err && err.message); - } - - // The rollup counts live permission rows, so it has to be refreshed after - // the grants above — the crawl and these writers must agree. - for (const row of rows) { - try { - await this.yp.await_proc("workspace_members_set", row.hub_id); - } catch (err) { - this.warn("[butler._resolve_pending_invitation] workspace member tracking failed for", row.hub_id, err && err.message); - } - } - await this.yp.await_proc("pending_invitation_delete_by_email", email); + const { resolvePendingInvitations } = require("./lib/resolve-pending-invitation"); + return resolvePendingInvitations(this, email, { source: "signup" }); } /** diff --git a/service/lib/resolve-pending-invitation.js b/service/lib/resolve-pending-invitation.js new file mode 100644 index 00000000..8cefc960 --- /dev/null +++ b/service/lib/resolve-pending-invitation.js @@ -0,0 +1,360 @@ +/** + * @license + * Copyright 2026 Thidima SA. All Rights Reserved. + * Licensed under the GNU AFFERO GENERAL PUBLIC LICENSE, Version 3. + * https://www.gnu.org/licenses/agpl-3.0.html + */ + +const { RedisStore, utils } = require("@drumee/server-essentials"); +const { isEmpty } = require("lodash"); + +// toArray from server-essentials, NOT lodash — lodash's turns an object into +// its values array, which mangles a single-row proc result (a list proc with +// exactly one row answers with the object itself). Same reason hub.js:52 and +// notify-member-joined.js pick it up from here. +const { toArray } = utils; +const { notifyMemberJoined } = require("./notify-member-joined"); + +/** + * REDEEM EVERY WORKSPACE INVITATION HELD FOR AN EMAIL ADDRESS. + * + * `yp.pending_invitation` is where an invitation waits when membership could + * not be granted at the time it was sent. Redeeming it is what actually joins + * the person to the workspace: `add_member` writes the hub's permission row AND + * calls `.join_hub`, which is what puts the workspace under their + * home root. Until that runs the invitee has NOTHING — `desk.home` lists their + * home's children, and the workspace is not among them, so it is invisible on + * their desk and stays invisible across reloads. + * + * WHY THIS IS A SHARED MODULE. Two byte-for-byte copies of this used to live in + * `signup.js` and `butler.js` — the two account-creation paths — and BOTH were + * only ever reachable at account creation. That left the case this module now + * also serves: an invitation addressed to somebody who ALREADY has an account. + * Nothing redeemed those, so they sat in the table forever. See the login hook + * in `yp.js` and `scripts/redeem-orphan-invitations.js`. + * + * NEVER THROWS PER HUB. One unreachable workspace must not cost the caller the + * others, and no caller (a sign-up, a login) may fail because of this — every + * failure is warned and swallowed. Rows that could not be redeemed are LEFT IN + * PLACE so a later attempt retries them; rows that are redeemed, expired, or + * point at a workspace that no longer exists are cleared. + * + * @param {Object} svc calling service instance — needs `yp`, `warn`, and + * `payload` when `notify` is on + * @param {String} email the invited address + * @param {Object} [opt] + * @param {String} [opt.uid] the invitee's drumate id when the caller already + * holds it (a login does) — saves a lookup + * @param {String} [opt.source] what triggered this, for the audit line + * @param {Boolean} [opt.notify] push over the socket + tell hub members + * (default true; off for offline scripts) + * @returns {Promise} { redeemed, failed, alreadyMember, hubs } + */ +async function resolvePendingInvitations(svc, email, opt = {}) { + const { source = "signup", notify = true } = opt; + const out = { redeemed: 0, failed: 0, alreadyMember: 0, hubs: [] }; + if (!svc || !svc.yp || isEmpty(email)) return out; + + const warn = (...a) => svc.warn && svc.warn("[pending-invitation]", ...a); + + // The lookup FIRST, and it is the whole cost on the common path: almost every + // caller has no pending row, and this returns empty without touching anything + // else. That is what makes it cheap enough to run on every login. + let rows; + try { + rows = toArray(await svc.yp.await_proc("pending_invitation_get_by_email", email)); + } catch (e) { + warn("lookup failed for", email, e && e.message); + return out; + } + if (isEmpty(rows)) return out; + + let uid = opt.uid || null; + if (!uid) { + try { + let user = await svc.yp.await_proc("drumate_exists", email); + if (Array.isArray(user)) user = user[0]; + uid = user && user.id; + } catch (e) { + warn("drumate_exists failed for", email, e && e.message); + return out; + } + } + // No account yet — the invitation is not orphaned, it is simply still + // waiting. Leave every row where it is: account creation redeems them. + if (!uid) return out; + + const { writeAudit } = require("../private/_audit"); + const now = Math.floor(Date.now() / 1000); + // Rows we are done with, redeemed or not — see the delete at the bottom. + let unresolved = 0; + + for (const row of rows) { + const hub_id = row && row.hub_id; + if (!hub_id) continue; + try { + // ⚠️ UNITS. `pending_invitation.expiry_time` holds an ABSOLUTE unix + // timestamp (yp_add_pending_invitation converts the caller's hours with + // TIMESTAMPADD before storing), while `add_member` and `permission_grant` + // both take HOURS FROM NOW and do that same conversion themselves. + // Handing the stored value straight to them — which both old copies of + // this code did — asks for a grant expiring in ~1.8 billion hours. It + // errs open so nothing was ever reported, but it is not what the inviter + // set, and it is why this converts back. + const stored = ~~row.expiry_time; + if (stored && stored <= now) { + // Expired before it was ever redeemed. Nothing to grant; the row is + // cleared with the rest so it stops being retried. + continue; + } + const hours = stored ? Math.max(1, Math.ceil((stored - now) / 3600)) : 0; + + const db_name = await svc.yp.await_func("get_db_name", hub_id); + if (!db_name) { + // The workspace is gone. Stale row, not a failure — clearing it is the + // repair, and keeping it would mean retrying a dead hub on every login. + warn("no db_name for hub", hub_id, "— dropping stale invitation"); + continue; + } + + // 🚨 DO NOT WALK OVER A MEMBERSHIP THAT ALREADY WORKS. + // + // A pending row is deleted only when it is redeemed, so a STALE one can + // sit beside a membership that was later granted properly through + // `hub.invite` — and this runs on every login. Re-granting from the stale + // row would write ITS privilege over the live one (an admin re-invited at + // 15 would be demoted back to the 4 the old invitation carried), and + // `add_member` also re-runs `join_hub`, whose REPLACE re-derives the + // home-root row's filename through `unique_filename` and resets its rank + // — renaming and reshuffling a workspace the user has had for months. + // + // So: already in, and the workspace already sitting under their home root + // → nothing to repair, just clear the row. `state` is read rather than + // assumed because "has permission" and "can see it" are two different + // writes inside add_member, and a half-applied one is exactly what this + // module exists to finish. + const state = await _membership(svc, db_name, hub_id, uid, warn); + if (state.permission > 0 && state.visible) { + out.alreadyMember++; + continue; + } + // Never downgrade on the repair path either: a live grant that is higher + // than the invitation's wins. + const permission = Math.max(~~row.permission, state.permission); + + const r = await svc.yp.await_proc( + `${db_name}.add_member`, uid, permission, hours + ); + // add_member answers with a db_name-carrying row only when it actually + // wrote the membership; anything else means it bailed (an unknown member, + // or the hub inviting itself) and permission_grant below would create a + // grant with no membership behind it. + if (!r || !r.db_name) { + warn("add_member wrote nothing for", uid, "on hub", hub_id); + unresolved++; + out.failed++; + continue; + } + + await svc.yp.await_proc( + `${db_name}.permission_grant`, + "*", uid, hours, permission, "system", + `Resolved from pending_invitation (${source})` + ); + + // Chat upload, the same grant `hub._grantMembership` makes. Neither old + // copy of this code made it, so a member who arrived through a pending + // invitation could not attach a file in the workspace chat. + try { + const mfs_home = await svc.yp.await_proc(`${db_name}.mfs_home`); + if (mfs_home && mfs_home.chat_upload_id) { + await svc.yp.await_proc( + `${db_name}.permission_grant`, + mfs_home.chat_upload_id, uid, 0, 4, + "no_traversal", "chat upload permission" + ); + } + } catch (e) { + warn("chat upload grant failed for hub", hub_id, e && e.message); + } + + await writeAudit(svc, { + db: db_name, + uid, + action: "invite_accepted", + category: "member", + notify_to: "admin", + entity_id: hub_id, + log: `Invite accepted — ${email} joined the workspace (${source})`, + }); + + // The notification row the invitee reads in the bell. Written by + // `_grantMembership` on the immediate-grant path and by nothing at all on + // this one, so a redeemed invitation used to arrive with no trace: the + // workspace simply appeared. + // + // AUTHORED BY THE WORKSPACE OWNER, not by whoever sent the invitation: + // `pending_invitation` stores only (hub_id, email, permission, expiry), + // so the inviter is not recoverable here. The owner is the truthful + // stand-in — the row must name SOMEBODY, and naming the invitee (the only + // id in hand) would render as "you invited yourself". Skipped rather than + // faked when even the owner cannot be read. + try { + // A plain read, not `get_hub`: that proc is keyed by vhost and takes a + // visitor as its second argument, so it answers nothing useful here. + let info = typeof svc.yp.await_query !== "function" ? null : await svc.yp.await_query( + "SELECT owner_id, name FROM hub WHERE id=?", hub_id + ); + if (Array.isArray(info)) info = info[0]; + const author = info && info.owner_id; + if (author) { + await svc.yp.await_proc( + "contact_log_activity", author, uid, "hub_invite_received", + { + hub_id, + hub_name: (info && info.name) || null, + permission, + } + ); + } + } catch (e) { + warn("activity log failed for hub", hub_id, e && e.message); + } + + out.redeemed++; + out.hubs.push(hub_id); + + if (notify) await _push(svc, db_name, hub_id, uid, warn); + } catch (err) { + unresolved++; + out.failed++; + warn("failed for hub", hub_id, err && err.message); + } + } + + if (!out.redeemed) { + // Nothing was granted, which is not the same as nothing being resolved: the + // rows may all have been expired, stale, or already honoured. Those are + // finished with. A FAILURE is not — and there is no per-row delete, so the + // blanket one is only safe when no row still wants retrying. + if (!unresolved) await _clear(svc, email, warn); + return out; + } + + // Viral loop: one acceptance stamps every row still open for the address, so + // it runs once rather than per hub — and BEFORE the delete below, which + // erases the only other evidence the invitations existed. Never throws: an + // uncounted acceptance beats a join that reports failure. + try { + await svc.yp.await_proc("invite_track_accept", email, uid); + } catch (e) { + warn("invite tracking failed for", email, e && e.message); + } + + // The rollup counts live permission rows, so it is refreshed after the grants + // — the crawl and these writers have to agree. + for (const hub_id of out.hubs) { + try { + await svc.yp.await_proc("workspace_members_set", hub_id); + } catch (e) { + warn("member tracking failed for", hub_id, e && e.message); + } + } + + if (!unresolved) await _clear(svc, email, warn); + return out; +} + +/** + * What the invitee already has in this workspace. + * + * TWO SEPARATE FACTS, because `add_member` writes them into two different + * databases and a half-applied one is exactly what this module repairs: + * + * permission — the membership row in the HUB's permission table + * (`resource_id = '*'`), which is what `user_permission` reads + * and what `mfs_show_node_by` filters `privilege > 0` on. + * visible — the hub's row under the invitee's own home root, written by + * `join_hub`. Without it `desk.home` has nothing to list, so the + * workspace does not appear however good the permission is. + * + * Read directly rather than through `user_permission`: that function walks + * parents and falls back to the anonymous grants, so it answers > 0 for a + * public workspace nobody was ever added to. Here the question is strictly + * "does this person hold a membership row". + * + * A read that fails answers "nothing" — which sends the caller down the grant + * path. That is the safe direction: the same hub DB is about to be written + * anyway, so a failure here surfaces there as a failure rather than as a + * silently skipped repair. + */ +async function _membership(svc, db_name, hub_id, uid, warn) { + const state = { permission: 0, visible: false }; + if (typeof svc.yp.await_query !== "function") return state; + try { + let row = await svc.yp.await_query( + `SELECT permission FROM \`${db_name}\`.permission + WHERE resource_id='*' AND entity_id=? LIMIT 1`, uid + ); + if (Array.isArray(row)) row = row[0]; + state.permission = ~~(row && row.permission); + } catch (e) { + warn("membership read failed for hub", hub_id, e && e.message); + return state; + } + if (!state.permission) return state; + try { + const own_db = await svc.yp.await_func("get_db_name", uid); + if (!own_db) return state; + let row = await svc.yp.await_query( + `SELECT id FROM \`${own_db}\`.media WHERE id=? LIMIT 1`, hub_id + ); + if (Array.isArray(row)) row = row[0]; + state.visible = !!(row && row.id); + } catch (e) { + warn("home-root read failed for hub", hub_id, e && e.message); + } + return state; +} + +/** + * Tell the invitee's own live session, then the workspace's other members. + * + * `hub.invite_received` is what the desk listens for — the sidebar restarts its + * list on it and the topbar switcher resyncs (ui-team desk/index.js + * `_onWorkspaceWsEvent`, desk/workspace-list `handleWsEvent`) — so an invitation + * redeemed while the recipient is looking at the desk lands without a reload. + */ +async function _push(svc, db_name, hub_id, uid, warn) { + try { + if (typeof svc.payload !== "function") return; + const hub = await svc.yp.await_proc(`${db_name}.mfs_access_node`, uid, hub_id); + if (hub) { + hub.ownpath = "/"; + hub.hub_id = hub.actual_hub_id; + hub.db_name = hub.actual_db; + const sockets = await svc.yp.await_proc("user_sockets", uid); + await RedisStore.sendData( + svc.payload(hub, { service: "hub.invite_received" }), sockets + ); + await RedisStore.sendData( + svc.payload(hub, { service: "hub.add_contributors" }), sockets + ); + } + } catch (e) { + warn("ws notify failed for hub", hub_id, e && e.message); + } + // Any admin with the member matrix open refetches instead of showing a list + // that is missing the person who just joined. + await notifyMemberJoined(svc, hub_id, uid); +} + +async function _clear(svc, email, warn) { + try { + await svc.yp.await_proc("pending_invitation_delete_by_email", email); + } catch (e) { + warn("cleanup failed for", email, e && e.message); + } +} + +module.exports = { resolvePendingInvitations }; diff --git a/service/private/hub.js b/service/private/hub.js index c19a8801..54a54e2e 100644 --- a/service/private/hub.js +++ b/service/private/hub.js @@ -738,7 +738,6 @@ class __private_hub extends Hub { this.output.data([]); return; } - let { domain_id } = this.user.toJSON(); let db_name = await this.yp.await_func("get_db_name", this.uid); if (!db_name) { this.warn("[hub] add_contributors: no contact db for hub", this.uid); @@ -748,29 +747,42 @@ class __private_hub extends Hub { let proc = `${db_name}.my_contact_exists`; for (let entity of users) { try { - let contact = await this.yp.await_proc(proc, 'entity', entity, '', ''); - if (!isEmpty(contact)) { - if (contact.status == "active") { - members.push(contact.uid); - } else { - await this.yp.await_proc( - "yp_add_pending_invitation", - this.hub.get(Attr.id), - expiry, - privilege, - entity - ); - await writeAudit(this, { - db: this.hub.get(Attr.db_name), - uid: this.uid, - action: 'invite_sent', - category: 'member', - notify_to: 'admin', - entity_id: this.hub.get(Attr.id), - log: `Invite sent to ${entity} for workspace '${hubname}'`, - }); - } - } else { + const contact = await this.yp.await_proc(proc, 'entity', entity, '', ''); + + // WHO GETS ADDED ON THE SPOT: anybody who already holds a Drumee + // account. Full stop. + // + // 🚨 THIS USED TO BE GATED ON THE INVITER. Membership was granted only + // when the address was an ACTIVE CONTACT of the caller, or when the + // account lived in the caller's own `domain_id`; everything else was + // written to `yp.pending_invitation` instead. Both tests describe the + // INVITER's address book and the INVITER's organisation — neither says + // anything about whether the invitee can hold a membership, and + // `hub.invite` (the popup, the folder access panel, the restricted + // permission panel) has never applied either of them. + // + // The domain half was the damaging one, because a pending row is only + // ever redeemed at ACCOUNT CREATION (signup / butler). Send it to an + // address that already has an account and nothing redeems it, ever: no + // add_member, so no `join_hub`, so no workspace under the invitee's + // home root, so `desk.home` never lists it and the workspace is + // invisible on their desk across reloads. Free accounts all sit in + // domain 1 until `org_provision` moves them, so ANY invitation from a + // provisioned org to a free user landed there — reported as "I was + // invited to a workspace and I cannot see it". + // + // Authorisation is the hub's permission table, which add_member and + // permission_grant below write. Domain equality is not, and was not, + // an access rule anywhere else in the stack. + let uid = null; + if (!isEmpty(contact) && contact.status == "active") uid = contact.uid; + // AND FALL THROUGH WHEN THE CONTACT CARRIES NO UID. An address-book row + // can be `active` with an empty `uid` (it is filled in when the contact + // is matched to an account), and the old code pushed that empty value + // into `members` — where add_member quietly bailed and the person was + // never added. Asking `drumate_exists` is the answer to the same + // question, one query later. + if (!uid) { let drumate = null; try { drumate = await this.yp.await_proc("drumate_exists", entity); @@ -778,53 +790,55 @@ class __private_hub extends Hub { } catch (e) { this.warn("[hub] add_contributors: drumate_exists for entity", entity, e); } - const sameDomain = drumate && drumate.domain_id != null && domain_id === drumate.domain_id; - if (sameDomain) { - members.push(drumate.id); - } else { - // Not a contact: register as pending invitation and send contact invitation. - // Two cases: (1) Email already has Drumee account → contact.invite sends in-app mail. - // (2) Email not in system → contact.invite sends signup/invite email with token. - await this.yp.await_proc( - "yp_add_pending_invitation", - this.hub.get(Attr.id), - expiry, - privilege, - entity - ); - await writeAudit(this, { - db: this.hub.get(Attr.db_name), - uid: this.uid, - action: 'invite_sent', - category: 'member', - notify_to: 'admin', - entity_id: this.hub.get(Attr.id), - log: `Invite sent to ${entity} for workspace '${hubname}'`, + if (drumate && drumate.id) uid = drumate.id; + } + + if (uid) { + members.push(uid); + continue; + } + + // NO ACCOUNT YET. Register the invitation so account creation grants it + // (signup's create_account -> _resolve_pending_invitation), and send + // the invitation email. + await this.yp.await_proc( + "yp_add_pending_invitation", + this.hub.get(Attr.id), + expiry, + privilege, + entity + ); + await writeAudit(this, { + db: this.hub.get(Attr.db_name), + uid: this.uid, + action: 'invite_sent', + category: 'member', + notify_to: 'admin', + entity_id: this.hub.get(Attr.id), + log: `Invite sent to ${entity} for workspace '${hubname}'`, + }); + const isEmail = typeof entity === "string" && entity.indexOf("@") !== -1; + if (isEmail) { + try { + const ContactPrivate = require("./contact"); + const contactSvc = new ContactPrivate({ + session: this.session, + permission: this.permission || { scope: "hub" } }); - const isEmail = typeof entity === "string" && entity.indexOf("@") !== -1; - if (isEmail) { - try { - const ContactPrivate = require("./contact"); - const contactSvc = new ContactPrivate({ - session: this.session, - permission: this.permission || { scope: "hub" } - }); - contactSvc.db = { - await_proc: (proc, ...args) => this.yp.await_proc(`${db_name}.${proc}`, ...args), - end: () => Promise.resolve() - }; - const origEmail = this.input.use(Attr.email) ?? this.input.get(Attr.email); - const origMessage = this.input.use(Attr.message); - this.input.set(Attr.email, entity); - this.input.set(Attr.message, message); - this.input.set("_contact_db_name", db_name); - await contactSvc.invite(); - if (origEmail !== undefined) this.input.set(Attr.email, origEmail); - if (origMessage !== undefined) this.input.set(Attr.message, origMessage); - } catch (err) { - this.warn("[hub] add_contributors: send invitation failed for entity", entity, err); - } - } + contactSvc.db = { + await_proc: (proc, ...args) => this.yp.await_proc(`${db_name}.${proc}`, ...args), + end: () => Promise.resolve() + }; + const origEmail = this.input.use(Attr.email) ?? this.input.get(Attr.email); + const origMessage = this.input.use(Attr.message); + this.input.set(Attr.email, entity); + this.input.set(Attr.message, message); + this.input.set("_contact_db_name", db_name); + await contactSvc.invite(); + if (origEmail !== undefined) this.input.set(Attr.email, origEmail); + if (origMessage !== undefined) this.input.set(Attr.message, origMessage); + } catch (err) { + this.warn("[hub] add_contributors: send invitation failed for entity", entity, err); } } } catch (err) { @@ -1925,7 +1939,6 @@ class __private_hub extends Hub { const username = this.user.get('fullname'); const lang = this.user.language() || this.input.app_language(); - const { domain_id } = this.user.toJSON(); const expiry = 0; // No expiry option in UI // Inviter's contact DB — used for my_contact_exists lookups @@ -1973,38 +1986,21 @@ class __private_hub extends Hub { // Resolve each user entity for (const entity of users) { try { - // 1. Check inviter's contact list + // 1. Resolve the entity to an ACCOUNT. Anybody who already holds one + // is granted membership on the spot — see the long note in + // add_contributors: the "active contact of the inviter, or same + // domain_id" test that used to stand here describes the INVITER, + // and a pending row addressed to an existing account is redeemed + // by nothing, so the workspace never appeared on their desk. const contact = await this.yp.await_proc(contact_proc, 'entity', entity, '', ''); - if (!isEmpty(contact)) { - if (contact.status === 'active') { - // Known active contact → add immediately - members.push(contact.uid); - memberEmail.set(contact.uid, contact.email || asEmail(entity)); - } else { - // Pending contact → store for deferred grant - await this.yp.await_proc( - 'yp_add_pending_invitation', - hub_id, expiry, privilege, entity - ); - await writeAudit(this, { - db: hub_db, - uid: this.uid, - action: 'invite_sent', - category: 'member', - notify_to: 'admin', - entity_id: hub_id, - log: `Invite sent to ${entity} for workspace '${hubname}'`, - }); - await this._trackInviteSent({ - hub_id, - email: asEmail(entity), - had_account: false, - source: "invite_with_roles", - }); - } + let uid = null; + let uidEmail = null; + if (!isEmpty(contact) && contact.status === 'active' && contact.uid) { + uid = contact.uid; + uidEmail = contact.email || asEmail(entity); } else { - // Not in contact list — check if user exists in Drumee + // Including an `active` contact with no uid — see add_contributors. let drumate = null; try { drumate = await this.yp.await_proc('drumate_exists', entity); @@ -2012,67 +2008,67 @@ class __private_hub extends Hub { } catch (e) { this.warn('[hub] invite_with_roles: drumate_exists failed for', entity, e); } + if (drumate && drumate.id) { + uid = drumate.id; + uidEmail = drumate.email || asEmail(entity); + } + } - const sameDomain = - drumate && - drumate.domain_id != null && - domain_id === drumate.domain_id; - - if (sameDomain) { - // Exists on same domain → add immediately - members.push(drumate.id); - memberEmail.set(drumate.id, drumate.email || asEmail(entity)); - } else { - // Unknown user or different domain → pending + invite email - await this.yp.await_proc( - 'yp_add_pending_invitation', - hub_id, expiry, privilege, entity - ); - await writeAudit(this, { - db: hub_db, - uid: this.uid, - action: 'invite_sent', - category: 'member', - notify_to: 'admin', - entity_id: hub_id, - log: `Invite sent to ${entity} for workspace '${hubname}'`, - }); - await this._trackInviteSent({ - hub_id, - email: asEmail(entity), - had_account: false, - source: "invite_with_roles", - }); + if (uid) { + members.push(uid); + memberEmail.set(uid, uidEmail); + continue; + } - // Only send email if entity looks like an email address - const isEmail = typeof entity === 'string' && entity.indexOf('@') !== -1; - if (isEmail) { - try { - const ContactPrivate = require('./contact'); - const contactSvc = new ContactPrivate({ - session: this.session, - permission: this.permission || { scope: 'hub' }, - }); - contactSvc.db = { - await_proc: (proc, ...args) => - this.yp.await_proc(`${contact_db}.${proc}`, ...args), - end: () => Promise.resolve(), - }; - const origEmail = this.input.use(Attr.email); - const origMessage = this.input.use(Attr.message); - this.input.set(Attr.email, entity); - this.input.set(Attr.message, msg); - this.input.set('_contact_db_name', contact_db); - await contactSvc.invite(); - if (origEmail !== undefined) this.input.set(Attr.email, origEmail); - if (origMessage !== undefined) this.input.set(Attr.message, origMessage); - } catch (err) { - this.warn( - '[hub] invite_with_roles: send invitation failed for', - entity, err - ); - } - } + // 2. No account yet → pending + invite email. Account creation + // redeems the row (signup -> _resolve_pending_invitation). + await this.yp.await_proc( + 'yp_add_pending_invitation', + hub_id, expiry, privilege, entity + ); + await writeAudit(this, { + db: hub_db, + uid: this.uid, + action: 'invite_sent', + category: 'member', + notify_to: 'admin', + entity_id: hub_id, + log: `Invite sent to ${entity} for workspace '${hubname}'`, + }); + await this._trackInviteSent({ + hub_id, + email: asEmail(entity), + had_account: false, + source: "invite_with_roles", + }); + + // Only send email if entity looks like an email address + const isEmail = typeof entity === 'string' && entity.indexOf('@') !== -1; + if (isEmail) { + try { + const ContactPrivate = require('./contact'); + const contactSvc = new ContactPrivate({ + session: this.session, + permission: this.permission || { scope: 'hub' }, + }); + contactSvc.db = { + await_proc: (proc, ...args) => + this.yp.await_proc(`${contact_db}.${proc}`, ...args), + end: () => Promise.resolve(), + }; + const origEmail = this.input.use(Attr.email); + const origMessage = this.input.use(Attr.message); + this.input.set(Attr.email, entity); + this.input.set(Attr.message, msg); + this.input.set('_contact_db_name', contact_db); + await contactSvc.invite(); + if (origEmail !== undefined) this.input.set(Attr.email, origEmail); + if (origMessage !== undefined) this.input.set(Attr.message, origMessage); + } catch (err) { + this.warn( + '[hub] invite_with_roles: send invitation failed for', + entity, err + ); } } } catch (err) { @@ -2144,7 +2140,17 @@ class __private_hub extends Hub { hub.hub_id = hub.actual_hub_id; hub.db_name = hub.actual_db; const sockets = await this.yp.await_proc('user_sockets', recipient.id); - await RedisStore.sendData(this.payload(hub), sockets); + // NAMED, like invite()'s and add_contributors' own pushes. An unnamed + // payload arrives at the desk with `options.service` undefined, and + // every consumer keys off exactly that: the dispatcher's default branch + // re-emits it as `ws:event`, the topbar switcher drops it on + // `if (!service) return`, and the sidebar's handler falls through to + // super. So a member added here was granted correctly and then had to + // reload to find out — the same invisible-workspace symptom, one layer + // up from the grant. + await RedisStore.sendData( + this.payload(hub, { service: "hub.invite_received" }), sockets + ); } // One broadcast per hub is enough — the matrix refetches the whole diff --git a/service/scripts/redeem-orphan-invitations.js b/service/scripts/redeem-orphan-invitations.js new file mode 100644 index 00000000..e4c2641e --- /dev/null +++ b/service/scripts/redeem-orphan-invitations.js @@ -0,0 +1,133 @@ +/** + * @license + * Copyright 2026 Thidima SA. All Rights Reserved. + * Licensed under the GNU AFFERO GENERAL PUBLIC LICENSE, Version 3. + * https://www.gnu.org/licenses/agpl-3.0.html + */ + +/** + * REPAIR WORKSPACE INVITATIONS THAT WERE NEVER GRANTED. + * + * An orphaned invitation is a `yp.pending_invitation` row whose address ALREADY + * BELONGS TO A DRUMEE ACCOUNT. Those rows are only ever redeemed at account + * creation, so for an existing account nothing redeemed them: no `add_member`, + * therefore no `join_hub`, therefore no workspace under the invitee's home root + * and nothing for `desk.home` to return. The workspace is invisible on their + * desk and stays invisible across reloads. + * + * `hub.add_contributors` / `hub.invite_with_roles` no longer create such rows + * (they grant to any existing account, whatever domain it is in), and + * `yp.login` redeems leftovers on the invitee's next sign-in. This script is + * the immediate, complete pass — it does not wait for people to log in. + * + * SAFE TO RE-RUN, and safe next to memberships that already work. Every row is + * checked before anything is written: somebody already holding the membership + * AND already carrying the workspace under their home root is left untouched + * (their stale invitation is simply cleared), and a repair never lowers a + * privilege that was raised since the invitation was sent. Rows that fail are + * kept for the next run. + * + * DRY RUN BY DEFAULT — it prints what it would do and writes nothing. + * + * Usage: + * node service/scripts/redeem-orphan-invitations.js # report only + * node service/scripts/redeem-orphan-invitations.js --apply # do it + * node service/scripts/redeem-orphan-invitations.js --apply --email=a@b.c + */ + +const { Mariadb } = require("@drumee/server-essentials"); +const { resolvePendingInvitations } = require("../lib/resolve-pending-invitation"); + +const argv = process.argv.slice(2); +const APPLY = argv.includes("--apply"); +const ONE = (argv.find((a) => a.startsWith("--email=")) || "").split("=")[1] || null; + +/** + * The shape `resolvePendingInvitations` needs. No `payload`, so it skips the + * websocket push — this runs outside any request and the recipients are + * overwhelmingly offline; their next desk load reads `desk.home` fresh anyway. + */ +function shim(yp) { + return { + yp, + warn: (...a) => console.warn(" !", ...a), + debug: () => {}, + }; +} + +async function main() { + const stamp = new Date().toISOString(); + console.log(`[${stamp}] orphaned workspace invitations — ${APPLY ? "APPLY" : "DRY RUN"}`); + + const yp = new Mariadb({ name: "yp" }); + try { + // Orphaned = the address already has an account. `drumate.email` and + // `pending_invitation.email` are both utf8mb4_general_ci, so the join is + // case-insensitive, which is what the redeemer's own lookup does too. + const sql = ` + SELECT p.email, d.id AS uid, COUNT(*) AS pending, MIN(p.created_at) AS oldest + FROM pending_invitation p + INNER JOIN drumate d ON d.email = p.email + ${ONE ? "WHERE p.email = ?" : ""} + GROUP BY p.email, d.id + ORDER BY oldest ASC + `; + const rows = (ONE ? await yp.await_query(sql, ONE) : await yp.await_query(sql)) || []; + // A one-row result comes back as the object itself, and a no-row one can + // come back as `{}` — filter on a field every real row has rather than on + // length, or an empty answer reads as one account with `undefined` rows. + const list = (Array.isArray(rows) ? rows : [rows]).filter((r) => r && r.email); + + if (!list.length) { + console.log("Nothing to repair."); + await yp.end(); + return; + } + + console.log(`${list.length} account(s) holding un-granted invitations:\n`); + let redeemed = 0; + let failed = 0; + // Rows whose workspace the person is already in — a stale invitation left + // behind by a later, successful invite. Cleared, never re-granted. + let skipped = 0; + + for (const row of list) { + const age = row.oldest + ? `${Math.floor((Date.now() / 1000 - row.oldest) / 86400)}d` + : "?"; + console.log(` ${row.email} (${row.pending} invitation(s), oldest ${age})`); + if (!APPLY) continue; + + const res = await resolvePendingInvitations(shim(yp), row.email, { + uid: row.uid, + source: "repair-script", + notify: false, + }); + redeemed += res.redeemed; + failed += res.failed; + skipped += res.alreadyMember; + console.log( + ` → redeemed ${res.redeemed}, already a member ${res.alreadyMember}` + + `, failed ${res.failed}` + ); + } + + console.log(""); + if (!APPLY) { + console.log("Dry run — nothing written. Re-run with --apply."); + } else { + console.log( + `Done: ${redeemed} membership(s) granted, ${skipped} already in place` + + `, ${failed} failure(s).` + ); + if (failed) console.log("Failed rows were LEFT IN PLACE — re-run to retry them."); + } + await yp.end(); + } catch (e) { + console.error("FAILED:", e && e.message); + try { await yp.end(); } catch (_) {} + process.exitCode = 1; + } +} + +main(); diff --git a/service/signup.js b/service/signup.js index f3ccff08..855133b4 100644 --- a/service/signup.js +++ b/service/signup.js @@ -16,11 +16,10 @@ */ const { - Attr, Messenger, Cache, sysEnv, uniqueId, RedisStore + Attr, Messenger, Cache, sysEnv, uniqueId } = require("@drumee/server-essentials"); const { resolve } = require('path'); const { isEmpty } = require("lodash"); -const { notifyMemberJoined } = require("./lib/notify-member-joined"); const { Mfs } = require("@drumee/server-core"); class __signup extends Mfs { @@ -147,92 +146,15 @@ class __signup extends Mfs { /** * Resolve pending hub invitations for a newly created user. - * Mirrors butler._resolve_pending_invitation. + * + * The body of this used to live here, duplicated byte-for-byte in butler.js, + * and reachable from account creation ONLY. Both copies now defer to + * `service/lib/resolve-pending-invitation`, which the login path and the + * repair script share — see that module for why that matters. */ async _resolve_pending_invitation(email) { - const { toArray } = require("@drumee/server-essentials").utils; - - const newUser = await this.yp.await_proc("drumate_exists", email); - if (isEmpty(newUser) || !newUser.id) return; - - const pending = await this.yp.await_proc("pending_invitation_get_by_email", email); - const rows = toArray(pending); - if (isEmpty(rows)) return; - - for (const row of rows) { - const { hub_id, permission, expiry_time } = row; - try { - const db_name = await this.yp.await_func("get_db_name", hub_id); - if (!db_name) continue; - await this.yp.await_proc(`${db_name}.add_member`, newUser.id, permission, expiry_time); - await this.yp.await_proc( - `${db_name}.permission_grant`, - '*', newUser.id, expiry_time, permission, 'system', 'Resolved from pending_invitation on signup' - ); - // Audit: invite redeemed on signup — never let a failure break the join. - try { - const { writeAudit } = require("./private/_audit"); - await writeAudit(this, { - db: db_name, - uid: newUser.id, - action: 'invite_accepted', - category: 'member', - notify_to: 'admin', - entity_id: hub_id, - log: `Invite accepted — ${email} joined the workspace on signup`, - }); - } catch (e) { /* audit only */ } - - // Notify the (now online) user so the desk sidebar can pick up the - // new workspace, mirroring hub.invite()'s notification for drumates. - try { - const hub = await this.yp.await_proc(`${db_name}.mfs_access_node`, newUser.id, hub_id); - if (hub) { - hub.ownpath = '/'; - hub.hub_id = hub.actual_hub_id; - hub.db_name = hub.actual_db; - const sockets = await this.yp.await_proc('user_sockets', newUser.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(`[signup._resolve_pending_invitation] WS notify failed for hub ${hub_id}:`, err && err.message); - } - - // Tell online members a member joined so an open permission matrix - // refreshes (mirrors butler.js _resolve_pending_invitation). - await notifyMemberJoined(this, hub_id, newUser.id); - } catch (err) { - this.warn(`[signup._resolve_pending_invitation] Failed for hub ${hub_id}:`, err && err.message); - } - } - - - // Viral loop: every pending invitation for this address is redeemed by this - // one sign-up, so the acceptance is recorded once for all of them rather - // than per hub — invite_track_accept stamps every row still open for the - // address. Placed BEFORE pending_invitation_delete_by_email below, which - // erases the only other evidence the invitations ever existed. - // - // Never allowed to throw: an account that was created and joined its - // workspaces, then reported a failure because a metric could not be - // written, is strictly worse than an uncounted acceptance. - try { - await this.yp.await_proc("invite_track_accept", email, newUser.id); - } catch (err) { - this.warn("[signup._resolve_pending_invitation] invite tracking failed for", email, err && err.message); - } - - // The rollup counts live permission rows, so it has to be refreshed after - // the grants above — the crawl and these writers must agree. - for (const row of rows) { - try { - await this.yp.await_proc("workspace_members_set", row.hub_id); - } catch (err) { - this.warn("[signup._resolve_pending_invitation] workspace member tracking failed for", row.hub_id, err && err.message); - } - } - await this.yp.await_proc("pending_invitation_delete_by_email", email); + const { resolvePendingInvitations } = require("./lib/resolve-pending-invitation"); + return resolvePendingInvitations(this, email, { source: "signup" }); } } diff --git a/service/yp.js b/service/yp.js index 5be5dc50..ea6bcc14 100644 --- a/service/yp.js +++ b/service/yp.js @@ -313,9 +313,67 @@ class __yp extends Entity { } await this._stampPasswordBacked(r, vars); + await this._redeemPendingInvitations(r, vars); this.output.data(r); } + /** + * SELF-HEAL AN INVITATION THAT WAS NEVER GRANTED. + * + * `yp.pending_invitation` holds invitations that could not be granted when + * they were sent, and until now the only thing that ever redeemed one was + * ACCOUNT CREATION. An invitation addressed to somebody who already had an + * account therefore sat there forever: no membership, no `join_hub`, so no + * workspace under their home root and nothing for `desk.home` to list — the + * workspace was simply invisible to them, reload after reload. + * + * The source of those rows is fixed (hub.add_contributors / invite_with_roles + * no longer refuse to grant across domains), so this is the net underneath: + * anyone already carrying an orphaned row is repaired the next time they sign + * in, with no support ticket and nothing for them to click. + * + * ONE INDEXED READ on the common path. `pending_invitation` is keyed by + * (hub_id, email) and almost every login finds nothing, in which case the + * helper returns before touching anything else. The uid is passed in so it + * does not have to look the account up again. + * + * NEVER FAILS A LOGIN. The session is already established by the time this + * runs; a repair that cannot complete is warned and retried on the next + * sign-in. + */ + async _redeemPendingInvitations(r, vars = {}) { + try { + if (!r || r.status !== "ok" || !r.user || !r.user.id) return; + const profile = isString(r.user.profile) + ? this.session.parseJSON(r.user.profile) + : (r.user.profile || {}); + let email = r.user.email + || profile.email + || (vars.uid && vars.uid.isEmail && vars.uid.isEmail() ? vars.uid : null); + // `pending_invitation` is keyed by ADDRESS, so without one there is + // nothing to look up. get_user's shape varies by path (the 2FA leg builds + // `r` from it directly), so resolve it rather than give up. + if (!email) { + let row = await this.yp.await_proc("drumate_exists", r.user.id); + if (isArray(row)) row = row[0]; + email = row && row.email; + } + if (!email) return; + const { resolvePendingInvitations } = require("./lib/resolve-pending-invitation"); + const res = await resolvePendingInvitations(this, email, { + uid: r.user.id, + source: "login", + }); + if (res.redeemed) { + this.debug( + "[login] redeemed", res.redeemed, "orphaned workspace invitation(s) for", email + ); + } + } catch (e) { + this.warn("login: pending invitation redemption failed:", e && e.message); + } + } + /** * A successful PASSWORD login is ground truth that the account holds a * real password. Legacy and OAuth-linked profiles may lack the @@ -402,6 +460,9 @@ class __yp extends Entity { } await logConnection(this, uid); const user = await this.yp.await_proc('get_user', uid); + // Same repair as the password path — 2FA accounts complete their sign-in + // here and never pass through login()'s tail. See _redeemPendingInvitations. + await this._redeemPendingInvitations({ status: "ok", user }); this.output.data(user); } diff --git a/test/pending-invitation-redeem.test.js b/test/pending-invitation-redeem.test.js new file mode 100644 index 00000000..dd2d74d4 --- /dev/null +++ b/test/pending-invitation-redeem.test.js @@ -0,0 +1,250 @@ +// Redeeming a workspace invitation that was never granted. +// +// THE REPORT: "I skipped creating a workspace during onboarding, a colleague +// invited me to theirs, and I cannot see it." The invitation had been written +// to yp.pending_invitation instead of being granted (hub.add_contributors +// refused to grant across domains, and a free account sits in domain 1 until +// org_provision moves it). Nothing redeems a pending row for an account that +// ALREADY EXISTS — both old redeemers ran at account creation only — so there +// was no add_member, therefore no join_hub, therefore no workspace under the +// invitee's home root and nothing for desk.home to list. Invisible, reload +// after reload. +// +// These lock the redeemer's contract: who gets granted, what is cleared, and +// what is deliberately left behind to be retried. +const assert = require('node:assert/strict'); +const test = require('node:test'); + +global.myDrumee = { arch: 'pod', useEmail: 0 }; +global.verbosity = 0; +global.debug = {}; + +const HOUR = 3600; +const NOW = () => Math.floor(Date.now() / 1000); + +// The lib destructures RedisStore once at load, so patching the method on that +// same object is what lets a test see the push without a live redis. +const essentials = require('@drumee/server-essentials'); +const sent = []; +essentials.RedisStore.sendData = async (payload, dest) => { sent.push({ payload, dest }); }; + +const { resolvePendingInvitations } = require('../service/lib/resolve-pending-invitation'); + +/** + * A yp double that records every call and answers the happy path by default. + * `answers` overrides one proc by name; `null` from an override means "no row". + */ +function fakeSvc(pending, answers = {}) { + const calls = []; + const pick = (name, args) => { + if (Object.prototype.hasOwnProperty.call(answers, name)) { + const a = answers[name]; + return typeof a === 'function' ? a(...args) : a; + } + return undefined; + }; + const yp = { + async await_proc(name, ...args) { + calls.push([name, ...args]); + const o = pick(name, args); + if (o !== undefined) return o; + if (name === 'pending_invitation_get_by_email') return pending; + if (/\.add_member$/.test(name)) return { id: 'u_invitee', db_name: 'a_invitee' }; + if (/\.mfs_home$/.test(name)) return { chat_upload_id: 'chat1' }; + if (/\.mfs_access_node$/.test(name)) return { actual_hub_id: args[1], actual_db: 'f_hub' }; + if (name === 'user_sockets') return [{ id: 's1' }]; + if (name === 'entity_sockets') return [{ id: 's2' }]; + return {}; + }, + async await_func(name, ...args) { + calls.push([name, ...args]); + const o = pick(name, args); + if (o !== undefined) return o; + if (name === 'get_db_name') return 'f_hub1'; + return null; + }, + // Routed by shape, because three different reads go through await_query: + // the live membership, the invitee's home-root row, and the hub's owner. + async await_query(sql, ...args) { + calls.push(['query', sql, ...args]); + if (/FROM `[^`]+`\.permission/.test(sql)) { + return answers.live === undefined ? [] : [{ permission: answers.live }]; + } + if (/FROM `[^`]+`\.media/.test(sql)) { + return answers.visible ? [{ id: args[0] }] : []; + } + return [{ owner_id: 'u_owner', name: 'Marketing' }]; + }, + }; + return { + yp, + calls, + warn: () => {}, + debug: () => {}, + payload: (data, opt) => ({ ...data, ...opt }), + }; +} + +const named = (svc, re) => svc.calls.filter((c) => re.test(c[0])); + +test('an address that already has an account is granted, in HOURS', async () => { + // The stored value is an ABSOLUTE timestamp (yp_add_pending_invitation + // converts hours with TIMESTAMPADD before storing) while add_member takes + // hours from now. Handing the stored number straight over — which both old + // copies did — asks for a grant expiring in ~1.8 billion hours. + const expires = NOW() + 48 * HOUR; + const svc = fakeSvc([{ hub_id: 'h1', email: 'a@b.c', permission: 6, expiry_time: expires }]); + + const res = await resolvePendingInvitations(svc, 'a@b.c', { uid: 'u_invitee' }); + + assert.equal(res.redeemed, 1); + const add = named(svc, /\.add_member$/)[0]; + assert.deepEqual(add.slice(0, 3), ['f_hub1.add_member', 'u_invitee', 6]); + assert.ok(add[3] >= 47 && add[3] <= 48, `expiry converted back to hours, got ${add[3]}`); + assert.equal(named(svc, /^pending_invitation_delete_by_email$/).length, 1, + 'a redeemed invitation is cleared'); +}); + +test('no expiry stays permanent', async () => { + const svc = fakeSvc([{ hub_id: 'h1', email: 'a@b.c', permission: 15, expiry_time: 0 }]); + await resolvePendingInvitations(svc, 'a@b.c', { uid: 'u_invitee' }); + assert.equal(named(svc, /\.add_member$/)[0][3], 0); +}); + +test('the membership grant is followed by the hub-root permission', async () => { + const svc = fakeSvc([{ hub_id: 'h1', email: 'a@b.c', permission: 6, expiry_time: 0 }]); + await resolvePendingInvitations(svc, 'a@b.c', { uid: 'u_invitee' }); + const grants = named(svc, /\.permission_grant$/); + assert.equal(grants[0][1], '*', 'membership is the star grant'); + assert.equal(grants[0][4], 6); + assert.ok(grants.some((g) => g[1] === 'chat1'), + 'chat upload too — neither old copy made this grant, so a member who ' + + 'arrived this way could not attach a file in the workspace chat'); +}); + +test('the invitee is told over the socket, on the name the desk listens for', async () => { + sent.length = 0; + const svc = fakeSvc([{ hub_id: 'h1', email: 'a@b.c', permission: 6, expiry_time: 0 }]); + await resolvePendingInvitations(svc, 'a@b.c', { uid: 'u_invitee' }); + assert.ok(sent.some((s) => s.payload.service === 'hub.invite_received'), + 'the switcher and the sidebar both key off this exact service name'); +}); + +test('notify:false keeps an offline repair silent', async () => { + sent.length = 0; + const svc = fakeSvc([{ hub_id: 'h1', email: 'a@b.c', permission: 6, expiry_time: 0 }]); + await resolvePendingInvitations(svc, 'a@b.c', { uid: 'u_invitee', notify: false }); + assert.equal(sent.length, 0); +}); + +test('an address with NO account is left alone for account creation', async () => { + const svc = fakeSvc( + [{ hub_id: 'h1', email: 'nobody@b.c', permission: 6, expiry_time: 0 }], + { drumate_exists: null } + ); + const res = await resolvePendingInvitations(svc, 'nobody@b.c'); + assert.equal(res.redeemed, 0); + assert.equal(named(svc, /\.add_member$/).length, 0); + assert.equal(named(svc, /^pending_invitation_delete_by_email$/).length, 0, + 'the invitation is still waiting — deleting it would lose it'); +}); + +test('an invitation that expired before redemption is dropped, not granted', async () => { + const svc = fakeSvc([ + { hub_id: 'h1', email: 'a@b.c', permission: 6, expiry_time: NOW() - HOUR }, + ]); + const res = await resolvePendingInvitations(svc, 'a@b.c', { uid: 'u_invitee' }); + assert.equal(res.redeemed, 0); + assert.equal(named(svc, /\.add_member$/).length, 0); + assert.equal(named(svc, /^pending_invitation_delete_by_email$/).length, 1, + 'nothing to grant and nothing to retry'); +}); + +test('an invitation into a workspace that no longer exists is dropped', async () => { + const svc = fakeSvc( + [{ hub_id: 'gone', email: 'a@b.c', permission: 6, expiry_time: 0 }], + { get_db_name: null } + ); + const res = await resolvePendingInvitations(svc, 'a@b.c', { uid: 'u_invitee' }); + assert.equal(res.redeemed, 0); + assert.equal(named(svc, /^pending_invitation_delete_by_email$/).length, 1, + 'a dead hub must not be retried on every login'); +}); + +test('a failure keeps the row so the next attempt retries it', async () => { + const svc = fakeSvc( + [{ hub_id: 'h1', email: 'a@b.c', permission: 6, expiry_time: 0 }], + { 'f_hub1.add_member': () => { throw new Error('hub db down'); } } + ); + const res = await resolvePendingInvitations(svc, 'a@b.c', { uid: 'u_invitee' }); + assert.equal(res.redeemed, 0); + assert.equal(res.failed, 1); + assert.equal(named(svc, /^pending_invitation_delete_by_email$/).length, 0, + 'a transient failure must not silently destroy the invitation'); +}); + +test('add_member writing nothing is a failure, not a silent success', async () => { + // It answers with a db_name-carrying row only when the membership was + // actually written; anything else means it bailed, and a permission_grant on + // top of that is a grant with no membership behind it. + const svc = fakeSvc( + [{ hub_id: 'h1', email: 'a@b.c', permission: 6, expiry_time: 0 }], + { 'f_hub1.add_member': { db_name: null } } + ); + const res = await resolvePendingInvitations(svc, 'a@b.c', { uid: 'u_invitee' }); + assert.equal(res.failed, 1); + assert.equal(named(svc, /\.permission_grant$/).length, 0); +}); + +test('an empty table costs one lookup and nothing else', async () => { + const svc = fakeSvc([]); + const res = await resolvePendingInvitations(svc, 'a@b.c', { uid: 'u_invitee' }); + assert.equal(res.redeemed, 0); + assert.equal(svc.calls.length, 1, + 'this runs on every login — the common path must stop at the lookup'); +}); + +// ── the guard against walking over a membership that already works ────────── +// +// A pending row is deleted only when it is redeemed, so a stale one outlives a +// membership later granted properly through hub.invite — and the redeemer runs +// on EVERY login. Without these it would re-grant from the stale row on each +// one: demoting a privilege that was raised since, and re-running join_hub, +// whose REPLACE re-derives the home-root filename and resets its rank. + +test('a stale invitation beside a working membership writes nothing', async () => { + const svc = fakeSvc( + [{ hub_id: 'h1', email: 'a@b.c', permission: 4, expiry_time: 0 }], + { live: 15, visible: true } + ); + const res = await resolvePendingInvitations(svc, 'a@b.c', { uid: 'u_invitee' }); + assert.equal(res.alreadyMember, 1); + assert.equal(res.redeemed, 0); + assert.equal(named(svc, /\.add_member$/).length, 0, + 'an admin at 15 must not be pushed back down to the invitation\'s 4'); + assert.equal(named(svc, /\.permission_grant$/).length, 0); + assert.equal(named(svc, /^pending_invitation_delete_by_email$/).length, 1, + 'the stale row is still cleared — that is the repair'); +}); + +test('permission without the home-root row is still repaired', async () => { + // add_member writes into two databases. Half-applied is precisely the state + // this module exists to finish: the grant is there, the workspace is not. + const svc = fakeSvc( + [{ hub_id: 'h1', email: 'a@b.c', permission: 4, expiry_time: 0 }], + { live: 4, visible: false } + ); + const res = await resolvePendingInvitations(svc, 'a@b.c', { uid: 'u_invitee' }); + assert.equal(res.redeemed, 1); + assert.equal(named(svc, /\.add_member$/).length, 1, 'join_hub has to be re-run'); +}); + +test('a repair never lowers a privilege raised since the invitation', async () => { + const svc = fakeSvc( + [{ hub_id: 'h1', email: 'a@b.c', permission: 4, expiry_time: 0 }], + { live: 15, visible: false } + ); + await resolvePendingInvitations(svc, 'a@b.c', { uid: 'u_invitee' }); + assert.equal(named(svc, /\.add_member$/)[0][2], 15); + assert.equal(named(svc, /\.permission_grant$/)[0][4], 15); +}); From 396e6f0042f35080e61cf8830206f9b5a2b6a309 Mon Sep 17 00:00:00 2001 From: EddyOne81 Date: Mon, 14 Sep 2026 04:17:15 -0700 Subject: [PATCH 02/15] fix(activity): a workspace invite reads "invited you to ", not "wants to connect" Lexis invited somebody into a workspace on prod and their notification said "Lexis wants to connect" -- the CONTACT-request sentence. A workspace invite is a yp.contact_activity row (event 'hub_invite_received', written by hub._grantMembership). Under Unread OFF, which is the panel's DEFAULT, the feed comes from activity_get_feed_all, whose contact branch returns every such row with `category` NULL and `event_type` 'contact'. The client resolves a row's category as `category || event_type || type`, so the invite resolved to 'contact' and took the contact branch of the row renderer -- the one that reads LOCALE.WANTS_TO_CONNECT. Under Unread ON the same event arrives from notification_hub_invites via mapHubInviteRow, which DOES set category 'hub_invite': the two toggle states disagreed and only the non-default one was right. task_assigned, task_mention and meeting_notice are contact_activity rows with exactly the same trap, and each dodges it with an `event ===` branch in the renderer that runs before the category switch. A hub invite has no such branch because the category it needs already exists -- it just never reached this path. _stampHubInvites stamps it server-side instead, so both toggle states now emit the identical row rather than the renderer growing a fourth special case. It is add-only, so the rollup rows merged into the same page (which already carry everything) pass through untouched: category 'hub_invite'. This also repairs the dismiss route: the client keys item_type off `category`, so these rows fell back to 'mfs' and dismissed a CHANGELOG id that happened to equal the contact_activity id -- a different table. They now dismiss as contact events, which is where the row lives. hub_id from the row's own `data` JSON. activity_get_feed_all sets hub_id NULL on every contact row, and the click needs it to open the workspace (it used to open the Contacts window). author_id the inviter, so the card shows their face instead of defaulting to the viewer's own. hub_name through resolveHubInviteName, the resolver already shared with notification_hub_invites and hub.invite_received_get, so a fourth copy of that chain cannot drift. The name has to be the LIVE one: hub.add_contributors and hub.invite_with_roles both record yp.hub.hubname -- the hex id -- as the invite's hub_name, and the resolver correctly refuses to render that as a label, so a stored-name-only fix would have left those rows blank. One push_workspace_name lookup per DISTINCT workspace, capped at 12, through _optionalYpProc so a deployment without that routine degrades to the invite-time name instead of warning on every feed load. Every failure is swallowed; an unnameable workspace loses its label, never the feed. Verified end to end against a real stage row (contact_activity 727, whose stored name IS the hex id): "temp test 1 wants to connect" becomes "temp test 1 invited you to Internal Workspace". offline/test/hub-invite-feed-row.test.js -- 47 cases and 10 negative controls over the real sliced method, including the identity proof that matters here: stamping a category does not move bookmarkKey(row), so an invitation the user had saved stays saved. All 38 suites green. Co-Authored-By: Claude Opus 5 (1M context) --- offline/test/hub-invite-feed-row.test.js | 555 +++++++++++++++++++++++ service/private/activity.js | 117 +++++ 2 files changed, 672 insertions(+) create mode 100644 offline/test/hub-invite-feed-row.test.js diff --git a/offline/test/hub-invite-feed-row.test.js b/offline/test/hub-invite-feed-row.test.js new file mode 100644 index 00000000..7935b3b6 --- /dev/null +++ b/offline/test/hub-invite-feed-row.test.js @@ -0,0 +1,555 @@ +#!/usr/bin/env node +// +// hub-invite-feed-row.test.js +// +// A workspace invitation must read " invited you to " in the +// chronological activity feed. It read " wants to connect" instead +// (Lexis, 2026-09-14) because activity_get_feed_all -- the Unread-OFF feed, which +// is the panel's DEFAULT -- returns the invite as a bare yp.contact_activity row: +// `category` NULL, `event_type` 'contact', `event` 'hub_invite_received'. The +// client resolves a row's category as `category || event_type || type`, so the +// row took the contact-request branch of the renderer. +// +// node offline/test/hub-invite-feed-row.test.js +// +// This exercises the REAL _stampHubInvites: the method is sliced out of +// service/private/activity.js and evaluated, together with the REAL +// _optionalYpProc guard and the REAL resolveHubInviteName resolver required from +// service/lib/. Nothing here is a copy -- a copy would keep passing after the +// service changed underneath it. +// +// It ends with negative controls: each one 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. +// +// 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 { resolveHubInviteName } = require('../../service/lib/hub-invite-name'); + +// The working tree must be byte-identical before and after this run. +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; +} + +// --------------------------------------------------------------------------- +// Slice the real code out of the service module. +// --------------------------------------------------------------------------- +function sliceMethod(marker, what) { + const start = src.indexOf(marker); + if (start < 0) { + console.error(`FATAL: could not find ${what} in ${SERVICE_FILE}`); + console.error('The marker moved — fix this slicer, do not weaken the test.'); + process.exit(1); + } + // Brace-match from the method's OWN `{`, not from the marker start. + 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 _stampHubInvites(', '_stampHubInvites'); +const GUARD_SRC = sliceMethod('\n async _optionalYpProc(', '_optionalYpProc'); + +// Sanity-check that we sliced what we think we did. A silent mis-slice would +// make every case below pass against the wrong code. +for (const needed of ['hub_invite_received', 'resolveHubInviteName', '_optionalYpProc', + 'push_workspace_name', 'MAX_LOOKUPS']) { + if (!STAMP_SRC.includes(needed)) { + console.error(`FATAL: sliced _stampHubInvites is missing ${needed} — slicer is wrong`); + process.exit(1); + } +} +if (!GUARD_SRC.includes('MISSING_PROCS')) { + console.error('FATAL: sliced _optionalYpProc does not consult MISSING_PROCS'); + process.exit(1); +} + +// Faithful to lib/utils/index.js toArray() for the shapes fed below. +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]; +}; + +// Build a stub `this` carrying the REAL method and the REAL guard. +// `names` maps hub_id -> workspace_name; a value of Error means the call throws, +// `undefined` means the routine is not deployed (await_proc answers undefined). +function harness({ names = {}, deployed = true, stampSrc = STAMP_SRC } = {}) { + const calls = []; + const impl = (new Function( + 'resolveHubInviteName', + `return { ${stampSrc} };`, + ))(resolveHubInviteName); + const guard = (new Function( + 'toArray', 'MISSING_PROCS', 'PROC_RETRY_MS', + `return { ${GUARD_SRC} };`, + // A fresh Map per harness: MISSING_PROCS is process-wide in production, but + // sharing it across cases would let one "not deployed" verdict silence the rest. + ))(toArray, new Map(), 60 * 1000); + + const ctx = { + calls, + debug() { }, + warn() { }, + yp: { + async await_proc(name, ...args) { + calls.push([name, ...args]); + if (!deployed) return undefined; + if (name !== 'push_workspace_name') return undefined; + const v = names[args[0]]; + if (v instanceof Error) throw v; + if (v === undefined) return []; + return [{ workspace_name: v }]; + }, + }, + _optionalYpProc: guard._optionalYpProc, + _stampHubInvites: impl._stampHubInvites, + }; + return ctx; +} + +const HUB = 'aaaa1111aaaa1112'; +const INVITER = 'bbbb2222bbbb2223'; + +// A row exactly as activity_get_feed_all's contact branch returns it. +function feedRow(over = {}) { + const meta = Object.assign({ + hub_id: HUB, + hub_name: 'Marketing', + message: 'Lexis added you to team Marketing.', + from_fullname: 'Lexis ', + privilege: 3, + }, over.meta || {}); + delete over.meta; + return Object.assign({ + id: 705, + timestamp: 1757308369, + uid: INVITER, + event: 'hub_invite_received', + event_type: 'contact', + data: JSON.stringify(meta), + is_read: 0, + firstname: 'Lexis', + lastname: '', + fullname: 'Lexis ', + hub_id: null, + hub_db_name: null, + category: null, + key_id: null, + last_id: null, + history_id: null, + }, over); +} + +// --------------------------------------------------------------------------- +// 1. The reported bug: the row must stop resolving to the contact category. +// --------------------------------------------------------------------------- +async function suite(stampSrc = STAMP_SRC, quiet = false) { + const before = { passed, failed }; + const C = quiet + ? (label, got, want) => { + const ok = JSON.stringify(got) === JSON.stringify(want); + if (ok) passed++; else { failed++; failures.push(`[NC] ${label}`); } + return ok; + } + : check; + + // The client resolves the category as `category || event_type || type`. + const clientCategory = (r) => r.category || r.event_type || r.type || null; + + { + const h = harness({ names: { [HUB]: 'Marketing' }, stampSrc }); + const rows = [feedRow()]; + await h._stampHubInvites.call(h, rows); + const r = rows[0]; + C('A1 category is stamped', r.category, 'hub_invite'); + C('A2 the client no longer resolves it as a contact request', + clientCategory(r), 'hub_invite'); + C('A3 workspace id is surfaced for the click target', r.hub_id, HUB); + C('A4 the inviter owns the avatar', r.author_id, INVITER); + C('A5 the sentence gets the workspace name', r.hub_name, 'Marketing'); + C('A6 event_type is untouched (the delete filter matches on it)', + r.event_type, 'contact'); + C('A7 the row id is untouched (dismiss + bookmark identity)', r.id, 705); + C('A8 no internal resolver field leaks into the published row shape', + 'hub_live_name' in r, false); + } + + // --------------------------------------------------------------------------- + // 2. The name chain. hub.add_contributors / hub.invite_with_roles record + // yp.hub.hubname -- the hex id -- as the invite's hub_name, so the stored + // name is not a usable label and the LIVE name has to win. + // --------------------------------------------------------------------------- + { + const h = harness({ names: { [HUB]: 'Marketing' }, stampSrc }); + const rows = [feedRow({ meta: { hub_name: HUB } })]; + await h._stampHubInvites.call(h, rows); + C('B1 a hex id stored as the name is replaced by the live name', + rows[0].hub_name, 'Marketing'); + } + { + // The live name WINS over a stale stored name (the workspace was renamed). + const h = harness({ names: { [HUB]: 'Growth' }, stampSrc }); + const rows = [feedRow()]; + await h._stampHubInvites.call(h, rows); + C('B2 a renamed workspace shows its current name', rows[0].hub_name, 'Growth'); + } + { + // Routine not applied on this deployment: degrade to the invite-time name. + const h = harness({ deployed: false, stampSrc }); + const rows = [feedRow()]; + await h._stampHubInvites.call(h, rows); + C('C1 an undeployed routine degrades to the stored name', + rows[0].hub_name, 'Marketing'); + C('C2 and the category is still fixed', rows[0].category, 'hub_invite'); + } + { + // Deployed but the workspace is gone (deleted): no live name, stored name wins. + const h = harness({ names: {}, stampSrc }); + const rows = [feedRow()]; + await h._stampHubInvites.call(h, rows); + C('C3 a deleted workspace falls back to the stored name', + rows[0].hub_name, 'Marketing'); + } + { + // Nothing resolves: render NO name rather than the hex id. + const h = harness({ names: {}, stampSrc }); + const rows = [feedRow({ meta: { hub_name: HUB } })]; + await h._stampHubInvites.call(h, rows); + C('D1 the hex id is never rendered as a workspace name', + rows[0].hub_name, undefined); + C('D2 the row is still an invitation', rows[0].category, 'hub_invite'); + } + { + const h = harness({ names: { [HUB]: ' Marketing ' }, stampSrc }); + const rows = [feedRow({ meta: { hub_name: HUB } })]; + await h._stampHubInvites.call(h, rows); + C('D3 the live name is trimmed', rows[0].hub_name, 'Marketing'); + } + { + const h = harness({ names: { [HUB]: ' ' }, stampSrc }); + const rows = [feedRow()]; + await h._stampHubInvites.call(h, rows); + C('D4 a blank live name is not a name', rows[0].hub_name, 'Marketing'); + } + + // --------------------------------------------------------------------------- + // 3. Row shapes the feed actually carries. + // --------------------------------------------------------------------------- + { + // Some drivers hand back `data` already parsed. + const h = harness({ names: { [HUB]: 'Marketing' }, stampSrc }); + const rows = [feedRow({ data: { hub_id: HUB, hub_name: 'Marketing' } })]; + await h._stampHubInvites.call(h, rows); + C('E1 an object `data` works like a string one', rows[0].hub_name, 'Marketing'); + C('E2 and still yields the workspace id', rows[0].hub_id, HUB); + } + { + const h = harness({ names: { [HUB]: 'Marketing' }, stampSrc }); + const rows = [feedRow({ data: '{not json' })]; + await h._stampHubInvites.call(h, rows); + C('F1 malformed JSON does not throw, and the row is still an invitation', + rows[0].category, 'hub_invite'); + C('F2 with no name rather than a wrong one', rows[0].hub_name, undefined); + C('F3 and no workspace id invented', rows[0].hub_id, null); + } + { + const h = harness({ names: { [HUB]: 'Marketing' }, stampSrc }); + const rows = [feedRow({ data: null })]; + await h._stampHubInvites.call(h, rows); + C('F4 a null `data` does not throw', rows[0].category, 'hub_invite'); + } + + // --------------------------------------------------------------------------- + // 4. ADD-ONLY. The Unread-ON path merges rollup rows that already carry all of + // this (mapHubInviteRow); they must pass through byte-identical. + // --------------------------------------------------------------------------- + { + const rollup = { + category: 'hub_invite', key_id: '705', hub_id: HUB, last_id: 705, cnt: 1, + ctime: 1757308369, firstname: 'Lexis', lastname: '', surname: 'Lexis ', + email: 'lexis@drumee.org', author_id: INVITER, hub_name: 'Marketing', + }; + const snapshot = JSON.stringify(rollup); + const h = harness({ names: { [HUB]: 'Renamed' }, stampSrc }); + await h._stampHubInvites.call(h, [rollup]); + C('G1 a rollup invite row is untouched', JSON.stringify(rollup), snapshot); + C('G2 and costs no lookup', h.calls.length, 0); + } + { + // A row that somehow already carries the fields keeps its own values. + const h = harness({ names: { [HUB]: 'Live' }, stampSrc }); + const rows = [feedRow({ hub_id: 'cccc3333cccc3334', author_id: 'dddd4444dddd4445' })]; + await h._stampHubInvites.call(h, rows); + C('G3 an existing hub_id is not overwritten', rows[0].hub_id, 'cccc3333cccc3334'); + C('G4 an existing author_id is not overwritten', rows[0].author_id, 'dddd4444dddd4445'); + } + + // --------------------------------------------------------------------------- + // 5. Every other row on the feed must come out exactly as it went in. + // --------------------------------------------------------------------------- + { + const others = [ + // a contact invitation — this is the row whose copy the bug borrowed + { + id: 700, uid: INVITER, event: 'contact.invite', event_type: 'contact', + category: null, hub_id: null, data: JSON.stringify({ msg: 'hi' }), + }, + // a task assignment (same table, own renderer branch) + { + id: 701, uid: INVITER, event: 'task_assigned', event_type: 'contact', + category: null, hub_id: null, + data: JSON.stringify({ hub_id: HUB, title: 'Ship it' }), + }, + // a scheduled-meeting notice + { + id: 702, uid: INVITER, event: 'meeting_notice', event_type: 'contact', + category: null, hub_id: null, + data: JSON.stringify({ hub_id: HUB, kind: 'invite', title: 'Standup' }), + }, + // an ordinary file event + { + id: 703, uid: INVITER, event: 'media.new', event_type: 'mfs', + category: null, hub_id: HUB, data: null, + }, + // an mfs_get_activity_feed row: no category, no event_type at all + { id: 704, uid: INVITER, event: 'media.share', hub_id: HUB }, + // the acceptance half of a contact handshake + { + id: 706, uid: INVITER, event: 'contact.accept_informed', event_type: 'contact', + category: null, hub_id: null, data: null, + }, + ]; + const snapshot = JSON.stringify(others); + const h = harness({ names: { [HUB]: 'Marketing' }, stampSrc }); + await h._stampHubInvites.call(h, others); + C('H1 no other feed row is touched', JSON.stringify(others), snapshot); + C('H2 and no lookup is spent on them', h.calls.length, 0); + } + + // --------------------------------------------------------------------------- + // 6. Cost. One lookup per DISTINCT workspace, hard-capped. + // --------------------------------------------------------------------------- + { + const A = 'aaaa0000aaaa0001'; + const B = 'bbbb0000bbbb0001'; + const rows = [ + feedRow({ id: 1, meta: { hub_id: A, hub_name: A } }), + feedRow({ id: 2, meta: { hub_id: A, hub_name: A } }), + feedRow({ id: 3, meta: { hub_id: B, hub_name: B } }), + feedRow({ id: 4, meta: { hub_id: A, hub_name: A } }), + feedRow({ id: 5, meta: { hub_id: B, hub_name: B } }), + ]; + const h = harness({ names: { [A]: 'Alpha', [B]: 'Beta' }, stampSrc }); + await h._stampHubInvites.call(h, rows); + C('I1 five rows over two workspaces cost two lookups', h.calls.length, 2); + C('I2 every row still gets its own name', + rows.map((r) => r.hub_name), ['Alpha', 'Alpha', 'Beta', 'Alpha', 'Beta']); + } + { + const many = []; + const names = {}; + for (let i = 0; i < 20; i++) { + const id = `hub${String(i).padStart(13, '0')}`; + names[id] = `WS${i}`; + many.push(feedRow({ id: 100 + i, meta: { hub_id: id, hub_name: `stored${i}` } })); + } + const h = harness({ names, stampSrc }); + await h._stampHubInvites.call(h, many); + C('I3 the lookup count is capped', h.calls.length <= 12, true); + C('I4 every row is still an invitation', + many.every((r) => r.category === 'hub_invite'), true); + C('I5 rows past the cap fall back to the stored name rather than going blank', + many.every((r) => typeof r.hub_name === 'string' && r.hub_name.length > 0), true); + } + + // --------------------------------------------------------------------------- + // 7. Failure never reaches the feed. + // --------------------------------------------------------------------------- + { + const h = harness({ names: { [HUB]: new Error('ER_LOCK_WAIT_TIMEOUT') }, stampSrc }); + const rows = [feedRow()]; + let threw = null; + try { await h._stampHubInvites.call(h, rows); } catch (e) { threw = e.message; } + C('J1 a throwing lookup is swallowed', threw, null); + C('J2 and the row degrades to the stored name', rows[0].hub_name, 'Marketing'); + C('J3 and is still an invitation', rows[0].category, 'hub_invite'); + } + { + const h = harness({ stampSrc }); + let threw = null; + try { + await h._stampHubInvites.call(h, []); + await h._stampHubInvites.call(h, null); + await h._stampHubInvites.call(h, [null, undefined]); + } catch (e) { threw = e.message; } + C('J4 empty / null / holey input does not throw', threw, null); + } + + // --------------------------------------------------------------------------- + // 8. Idempotence — the method runs once per feed today, but a second pass must + // change nothing (that is what makes it safe to move in the pipeline). + // --------------------------------------------------------------------------- + { + const h = harness({ names: { [HUB]: 'Marketing' }, stampSrc }); + const rows = [feedRow()]; + await h._stampHubInvites.call(h, rows); + const once = JSON.stringify(rows); + await h._stampHubInvites.call(h, rows); + C('K1 a second pass is a no-op', JSON.stringify(rows), once); + } + + // --------------------------------------------------------------------------- + // 9. Identity must not move. A saved ("bookmarked") notification is addressed + // by bookmarkKey(row), and stamping a category changes what that function + // reads. If the hash moved, every invitation a user had saved would come + // back unsaved. Run against the REAL exported function. + // --------------------------------------------------------------------------- + { + const Activity = require('../../service/private/activity'); + const raw = feedRow(); + const preFix = Activity.bookmarkKey(raw); + const h = harness({ names: { [HUB]: 'Marketing' }, stampSrc }); + await h._stampHubInvites.call(h, [raw]); + const postFix = Activity.bookmarkKey(raw); + const rollup = Activity.bookmarkKey({ + category: 'hub_invite', key_id: '705', hub_id: HUB, + }); + C('M1 the saved-notification identity does not move', postFix, preFix); + C('M2 and still matches the Unread-ON representation of the same invite', + postFix, rollup); + C('M3 (a real sha256, not two undefineds comparing equal)', + typeof postFix === 'string' && postFix.length === 64, true); + } + + // --------------------------------------------------------------------------- + // 10. Anti-drift invariants, asserted against the SOURCE. + // + // These three cannot be reached by feeding rows in -- no producer emits a row + // that would exercise them -- but each is a contract a later edit could break + // silently, so they are checked statically rather than not at all. + // --------------------------------------------------------------------------- + { + C('L1 the category stamp stays add-only (never clobbers a row that has one)', + /if\s*\(!r\.category\)\s*r\.category\s*=\s*'hub_invite'/.test(stampSrc), true); + C('L2 event_type is never rewritten (the delete filter matches on it)', + /r\.event_type\s*=/.test(stampSrc), false); + C('L3 the name comes only from the shared resolver, never an inline chain', + (stampSrc.match(/r\.hub_name\s*=(?!=)/g) || []).length === 1 + && /r\.hub_name\s*=\s*name\s*;/.test(stampSrc), true); + C('L4 get_feed actually calls it (otherwise the whole fix is dead code)', + /await this\._stampHubInvites\(result\);/.test(src), true); + } + + return { added: passed - before.passed + (failed - before.failed) }; +} + +// --------------------------------------------------------------------------- +// Negative controls. Each reintroduces one specific mistake into the sliced +// SOURCE and asserts the suite goes red. A control that stays green means the +// case it targets is not really testing that line. +// --------------------------------------------------------------------------- +async function negativeControls() { + const controls = [ + ['NC1 category is never stamped', + (s) => s.replace("if (!r.category) r.category = 'hub_invite';", '')], + ['NC2 category is stamped unconditionally (breaks the add-only contract)', + (s) => s.replace("if (!r.category) r.category = 'hub_invite';", "r.category = 'hub_invite';")], + ['NC2b the name is built inline instead of through the shared resolver', + (s) => s.replace('if (name) r.hub_name = name;', + "r.hub_name = live || (meta.hub_name !== meta.hub_id ? meta.hub_name : null);")], + ['NC3 the workspace id is not surfaced', + (s) => s.replace('if (r.hub_id == null && meta.hub_id != null) r.hub_id = meta.hub_id;', '')], + ['NC4 the inviter is not surfaced', + (s) => s.replace('if (r.author_id == null && r.uid != null) r.author_id = r.uid;', '')], + ['NC5 the event guard is dropped (every contact row becomes an invite)', + (s) => s.replace("if (!r || r.event !== 'hub_invite_received') continue;", 'if (!r) continue;')], + ['NC6 the live name never reaches the resolver (stored name wins)', + (s) => s.replace('live ? Object.assign({}, r, { hub_live_name: live }) : r,', 'r,')], + ['NC7 the lookup cap is removed', + (s) => s.replace('&& wanted.size < MAX_LOOKUPS', '')], + ['NC8 lookups are not deduped per workspace', + (s) => s.replace('const wanted = new Set();', 'const wanted = [];') + .replace('wanted.add(meta.hub_id);', 'wanted.push(meta.hub_id);') + .replace('wanted.size < MAX_LOOKUPS', 'wanted.length < MAX_LOOKUPS') + .replace('if (!targets.length) return;', 'if (!targets.length) return;')], + ]; + + let caught = 0; + + // L4 is asserted against the whole file rather than the sliced method, so it + // gets its own control: strip the call out of a COPY of the file and prove the + // invariant notices. + { + const stripped = src.replace(/\n\s*await this\._stampHubInvites\(result\);/, ''); + const noticed = stripped !== src && !/await this\._stampHubInvites\(result\);/.test(stripped); + if (noticed) { caught++; console.log(' ✓ NC9 get_feed no longer calls it — CAUGHT'); } + else { + console.log(' ✗ NC9 get_feed no longer calls it — NOT CAUGHT'); + failed++; failures.push('NC9 — NOT CAUGHT'); + } + } + + for (const [label, mutate] of controls) { + const mutated = mutate(STAMP_SRC); + if (mutated === STAMP_SRC) { + console.log(` ✗ ${label} — MUTATION DID NOT APPLY (the slicer or the code moved)`); + failed++; + failures.push(`${label}: mutation did not apply`); + continue; + } + const before = { passed, failed }; + let broke = false; + try { + await suite(mutated, true); + broke = failed > before.failed; + } catch (e) { + broke = true; // a crash is a failure too + } + // Roll the counters back: a negative control's own result is not a test result. + passed = before.passed; + failed = before.failed; + failures.length = failures.length; // (failures pushed by the quiet suite are dropped below) + while (failures.length && failures[failures.length - 1].startsWith('[NC]')) failures.pop(); + if (broke) { caught++; console.log(` ✓ ${label} — CAUGHT`); } + else { console.log(` ✗ ${label} — NOT CAUGHT`); failed++; failures.push(label + ' — NOT CAUGHT'); } + } + return caught; +} + +(async () => { + console.log('hub-invite feed row — "invited you to ", not "wants to connect"\n'); + await suite(); + console.log('\nnegative controls (each must break the suite):'); + const caught = await negativeControls(); + + const after = require('crypto').createHash('md5') + .update(fs.readFileSync(SERVICE_FILE, 'utf8')).digest('hex'); + check('Z1 the working tree was never modified', after, MD5_BEFORE); + + console.log(''); + for (const f of failures) console.log(' ✗ ' + f); + console.log(`\n${passed} passed, ${failed} failed, ${caught} negative controls caught.`); + process.exit(failed ? 1 : 0); +})(); diff --git a/service/private/activity.js b/service/private/activity.js index 22eba6cd..e33eba2f 100644 --- a/service/private/activity.js +++ b/service/private/activity.js @@ -1152,6 +1152,12 @@ class MfsActivity extends Entity { // targeted invitation already covers), so a failure anywhere leaves the feed // exactly as it renders today. await this._stampTaskFolderNames(result); + // Workspace invites: give the raw contact_activity row the category, the + // workspace id and the workspace NAME it needs to render as an invitation + // rather than as a contact request. See _stampHubInvites -- it is add-only, + // 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._stampChatMentions(result); result = await this._stampMeetingRollups(result); @@ -1448,6 +1454,117 @@ class MfsActivity extends Entity { } } + /** + * Make a workspace invitation READ as one in the chronological feed. + * + * THE BUG (Lexis, 2026-09-14): "[Username] invited you to [workspace]" was + * rendered as "[Username] wants to connect" -- the CONTACT-request copy. + * + * WHY. A workspace invite is a yp.contact_activity row (event + * 'hub_invite_received', written by hub._grantMembership). Under Unread OFF -- + * the panel's DEFAULT -- the feed comes from activity_get_feed_all, whose + * contact branch returns every such row with `category` NULL and `event_type` + * 'contact'. The client resolves a row's category as + * `category || event_type || type`, so the invite resolved to 'contact' and + * took the contact branch of the row renderer. Under Unread ON the same event + * arrives from notification_hub_invites via mapHubInviteRow, which DOES set + * category 'hub_invite' -- so the two toggle states disagreed, and only the + * non-default one was right. + * + * task_assigned / task_mention / meeting_notice are contact_activity rows with + * exactly the same problem, and each dodges it with an `event ===` branch that + * runs BEFORE the category switch. A hub invite has no such branch, because + * the category it needs already exists -- it just never reached this path. + * Stamping it here makes both toggle states produce the identical row, instead + * of adding a fourth special case to the renderer. + * + * Three things are stamped, all of them ADD-ONLY (a row that already carries + * the field -- i.e. the rollup row -- is never touched, so this is idempotent + * and cannot disturb the Unread-ON path): + * + * category 'hub_invite'. Also repairs the dismiss route: the client keys + * item_type off `category`, so these rows used to fall back to + * 'mfs' and dismiss a CHANGELOG id that happened to equal the + * contact_activity id. Now they dismiss as contact events, which + * is the table the row actually lives in. + * hub_id the invited workspace, from the row's own `data` JSON. + * activity_get_feed_all sets hub_id NULL on every contact row, + * and the invite row is the one place the client needs it (the + * click opens that workspace). Same trick flattenTaskFields uses. + * author_id the inviter, so the card shows THEIR face. Without it + * getAuthorId() falls through to undefined and the avatar + * defaults to the viewer's own picture. + * + * The workspace NAME goes through resolveHubInviteName, the resolver shared + * with notification_hub_invites and hub.invite_received_get, so a fourth copy + * of that chain cannot drift (that drift is what once left the name blank). + * Its first real term is the LIVE name, which is resolved here per distinct + * workspace: `data.hub_name` alone is not enough, because hub.add_contributors + * and hub.invite_with_roles both record yp.hub.hubname -- the hex id -- and the + * resolver correctly refuses to render that as a label. + * + * Best-effort throughout, exactly like _stampFolderNames: one lookup per + * DISTINCT workspace, a hard cap, every failure swallowed, and a missing + * routine degrades to the invite-time name rather than breaking the feed. + * + * Covered by offline/test/hub-invite-feed-row.test.js, which runs this method + * for real and holds the identity proof that matters here: stamping a category + * does NOT move bookmarkKey(row), so an invitation the user had saved stays + * saved. + */ + async _stampHubInvites(rows) { + const MAX_LOOKUPS = 12; + if (!Array.isArray(rows) || !rows.length) return; + + const targets = []; // [row, meta] + const wanted = new Set(); + for (const r of rows) { + if (!r || r.event !== 'hub_invite_received') continue; + let meta = r.data; + if (typeof meta === 'string') { + try { meta = JSON.parse(meta); } catch (e) { meta = null; } + } + meta = meta || {}; + 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; + targets.push([r, meta]); + if (r.hub_name == null && meta.hub_id && wanted.size < MAX_LOOKUPS) { + wanted.add(meta.hub_id); + } + } + if (!targets.length) return; + + const names = new Map(); + for (const hubId of wanted) { + try { + // Read-only, one row, yp.hub.name -- the name members set and see. + // _optionalYpProc, not await_proc: on a deployment where the routine is + // not applied yet this must degrade to the invite-time name, without a + // warning on every feed load. + const row = (await this._optionalYpProc('push_workspace_name', hubId))[0]; + const name = row && String(row.workspace_name || '').trim(); + if (name) names.set(hubId, name); + } catch (e) { + this.debug('[ACTIVITY] workspace name lookup failed', hubId, e && e.message); + } + } + + for (const [r, meta] of targets) { + if (r.hub_name != null) continue; + // `hub_live_name` is the resolver's own term for the live name. It rides a + // shallow copy rather than being stamped on the row: the client never + // reads it, and the feed row's shape is a published contract + // (acl/activity.json). + const live = meta.hub_id ? names.get(meta.hub_id) : null; + const name = resolveHubInviteName( + live ? Object.assign({}, r, { hub_live_name: live }) : r, + meta, + ); + if (name) r.hub_name = name; + } + } + /** * Turn a scheduled meeting's rollup row into a MEETING row (Duy 2026-08-21, * issues 9 + 10). From 9420a481d5572582795cd3706a23306a489629f2 Mon Sep 17 00:00:00 2001 From: EddyOne81 Date: Mon, 14 Sep 2026 04:20:36 -0700 Subject: [PATCH 03/15] fix(test): drop a no-op self-assignment the lint gate caught `failures.length = failures.length` was a leftover from an earlier shape of the negative-control rollback; the `while ... pop()` on the next line is what actually drops the control's own messages. no-self-assign is one of the correctness rules the Code Quality workflow runs on changed files, and it was right to flag it. Verified with the workflow's exact ESLint 9.39.5 ruleset: clean. 47 cases + 10 negative controls still pass. Co-Authored-By: Claude Opus 5 (1M context) --- offline/test/hub-invite-feed-row.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/offline/test/hub-invite-feed-row.test.js b/offline/test/hub-invite-feed-row.test.js index 7935b3b6..0230f999 100644 --- a/offline/test/hub-invite-feed-row.test.js +++ b/offline/test/hub-invite-feed-row.test.js @@ -530,7 +530,7 @@ async function negativeControls() { // Roll the counters back: a negative control's own result is not a test result. passed = before.passed; failed = before.failed; - failures.length = failures.length; // (failures pushed by the quiet suite are dropped below) + // Drop the messages the quiet suite pushed: a control's own red is expected. while (failures.length && failures[failures.length - 1].startsWith('[NC]')) failures.pop(); if (broke) { caught++; console.log(` ✓ ${label} — CAUGHT`); } else { console.log(` ✗ ${label} — NOT CAUGHT`); failed++; failures.push(label + ' — NOT CAUGHT'); } From 9a8d0d6d842ce7130beaa6756a535c49cda6d337 Mon Sep 17 00:00:00 2001 From: EddyOne81 Date: Mon, 14 Sep 2026 04:25:04 -0700 Subject: [PATCH 04/15] test(activity): prove stamping a category does not move the invite's tab MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit bucketOf() reads `category` before `event_type`, so _stampHubInvites could have relocated every workspace invitation to a different Notification Center tab without anything failing. The mapper is now sliced out of the same module (the block notification-bucket.test.js uses) and run before and after the stamp: same tab, and that tab is Other. 'other' is also bucketOf's own fallback, so a mapper that answered 'other' to everything would have passed both assertions — N3 makes it prove it discriminates first. 50 cases, 10 negative controls. Co-Authored-By: Claude Opus 5 (1M context) --- offline/test/hub-invite-feed-row.test.js | 39 ++++++++++++++++++++++-- 1 file changed, 36 insertions(+), 3 deletions(-) diff --git a/offline/test/hub-invite-feed-row.test.js b/offline/test/hub-invite-feed-row.test.js index 0230f999..2a52665a 100644 --- a/offline/test/hub-invite-feed-row.test.js +++ b/offline/test/hub-invite-feed-row.test.js @@ -444,10 +444,43 @@ async function suite(stampSrc = STAMP_SRC, quiet = false) { } // --------------------------------------------------------------------------- - // 10. Anti-drift invariants, asserted against the SOURCE. + // 10. The tab must not move. bucketOf() reads `category` before `event_type`, + // so stamping one could have relocated every workspace invitation to a + // different Notification Center tab. Run against the REAL mapper, sliced + // out of the same module (the same block notification-bucket.test.js uses). + // --------------------------------------------------------------------------- + { + const bStart = src.indexOf('\nconst BUCKET = {'); + const bEnd = src.indexOf('\nfunction validBucket(value) {'); + if (bStart < 0 || bEnd < 0 || bEnd < bStart) { + console.error('FATAL: could not locate the bucket block — fix this slicer'); + process.exit(1); + } + const block = src.slice(bStart, bEnd); + if (!block.includes('function bucketOf(row)')) { + console.error('FATAL: sliced bucket block has no bucketOf — slicer is wrong'); + process.exit(1); + } + // Newline, not `;` — the block ends on a line comment, which would swallow it. + const bucketOf = (new Function(`${block}\nreturn bucketOf;`))(); + + const raw = feedRow(); + const preFix = bucketOf(raw); + const h = harness({ names: { [HUB]: 'Marketing' }, stampSrc }); + await h._stampHubInvites.call(h, [raw]); + C('N1 the invitation stays in the same tab', bucketOf(raw), preFix); + C('N2 which is Other, the tab that owns member invites', preFix, 'other'); + // 'other' is also bucketOf's default, so N1/N2 could both pass against a + // mapper that answered 'other' to everything. Prove it discriminates. + C('N3 (the mapper is not a constant)', + bucketOf({ event: 'media.new', event_type: 'mfs' }), 'files'); + } + + // --------------------------------------------------------------------------- + // 11. Anti-drift invariants, asserted against the SOURCE. // - // These three cannot be reached by feeding rows in -- no producer emits a row - // that would exercise them -- but each is a contract a later edit could break + // These cannot be reached by feeding rows in -- no producer emits a row that + // would exercise them -- but each is a contract a later edit could break // silently, so they are checked statically rather than not at all. // --------------------------------------------------------------------------- { From cfb632f671c8d4a02ba38c3cbe9ee59822976669 Mon Sep 17 00:00:00 2001 From: luongtrieuvy202 Date: Mon, 14 Sep 2026 18:46:00 +0700 Subject: [PATCH 05/15] Revert "fix(hub): grant workspace invitations to existing accounts in any domain" This reverts commit 00b4483a1e94ee44e1f9a26e8ae87e9c22a4c0d1. Reverted on request. Restores the domain/contact gate in hub.add_contributors and hub.invite_with_roles, the two in-place copies of _resolve_pending_invitation in signup.js and butler.js, the unnamed websocket push in invite_with_roles, and removes the shared redeemer, the login hook, the repair script and its tests. The unrelated work that landed on top of it (notifyMembersChanged in delete_contributor / set_privilege) is untouched. Co-Authored-By: Claude Opus 5 (1M context) --- service/butler.js | 113 +++++- service/lib/resolve-pending-invitation.js | 360 ------------------- service/private/hub.js | 326 +++++++++-------- service/scripts/redeem-orphan-invitations.js | 133 ------- service/signup.js | 94 ++++- service/yp.js | 61 ---- test/pending-invitation-redeem.test.js | 250 ------------- 7 files changed, 351 insertions(+), 986 deletions(-) delete mode 100644 service/lib/resolve-pending-invitation.js delete mode 100644 service/scripts/redeem-orphan-invitations.js delete mode 100644 test/pending-invitation-redeem.test.js diff --git a/service/butler.js b/service/butler.js index ccf427b8..27df3e01 100644 --- a/service/butler.js +++ b/service/butler.js @@ -16,12 +16,13 @@ */ const { - Attr, Constants, sendSms, Messenger, Cache, sysEnv, uniqueId + Attr, Constants, sendSms, Messenger, Cache, sysEnv, uniqueId, RedisStore } = require("@drumee/server-essentials"); const { platform } = require('./lib/env'); const { logConnection } = require('./lib/connection_log'); const { resolve } = require('path'); const { isEmpty, isString } = require("lodash"); +const { notifyMemberJoined } = require("./lib/notify-member-joined"); const { PASS_CHECKER, ID_NOBODY, @@ -707,16 +708,112 @@ class __butler extends Mfs { * After a new user completes signup, resolve any pending hub invitations * that were registered via yp_add_pending_invitation before the account existed. * - * The body of this used to live here, duplicated byte-for-byte in signup.js, - * and reachable from account creation ONLY. Both copies now defer to - * `service/lib/resolve-pending-invitation`, which the login path and the - * repair script share — see that module for why that matters. - * * @param {string} email - newly registered user's email */ async _resolve_pending_invitation(email) { - const { resolvePendingInvitations } = require("./lib/resolve-pending-invitation"); - return resolvePendingInvitations(this, email, { source: "signup" }); + const { toArray } = require("@drumee/server-essentials").utils; + const { isEmpty } = require("lodash"); + + // 1. Get new user's ID + const newUser = await this.yp.await_proc("drumate_exists", email); + if (isEmpty(newUser) || !newUser.id) { + this.warn("[_resolve_pending_invitation] Cannot find newly created user for", email); + return; + } + + // 2. Find all pending hub invitations for this email + const pending = await this.yp.await_proc("pending_invitation_get_by_email", email); + const rows = toArray(pending); + if (isEmpty(rows)) return; + + for (const row of rows) { + const { hub_id, permission, expiry_time } = row; + try { + // 3. Get hub db_name explicitly + const db_name = await this.yp.await_func("get_db_name", hub_id); + if (!db_name) { + this.warn(`[_resolve_pending_invitation] Cannot find db_name for hub ${hub_id}`); + continue; + } + + // 4. Add as real member + await this.yp.await_proc(`${db_name}.add_member`, newUser.id, permission, expiry_time); + + // 5. Grant hub permission + await this.yp.await_proc( + `${db_name}.permission_grant`, + '*', newUser.id, expiry_time, permission, 'system', 'Resolved from pending_invitation on signup' + ); + + // Audit: invite redeemed on signup — never let a failure break the join. + try { + const { writeAudit } = require("./private/_audit"); + await writeAudit(this, { + db: db_name, + uid: newUser.id, + action: 'invite_accepted', + category: 'member', + notify_to: 'admin', + entity_id: hub_id, + log: `Invite accepted — ${email} joined the workspace on signup`, + }); + } catch (e) { /* audit only */ } + + // 6. Notify the (now online) user so the desk sidebar can pick up the + // new workspace, mirroring hub.invite()'s notification for drumates. + try { + const hub = await this.yp.await_proc(`${db_name}.mfs_access_node`, newUser.id, hub_id); + if (hub) { + hub.ownpath = '/'; + hub.hub_id = hub.actual_hub_id; + hub.db_name = hub.actual_db; + const sockets = await this.yp.await_proc('user_sockets', newUser.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(`[_resolve_pending_invitation] WS notify failed for hub ${hub_id}:`, err && err.message); + } + + // Tell online members a member joined so an open permission matrix + // refreshes. (butler.js has no direct service boundary with hub.js, so + // the shared helper is used instead of an inherited method.) + await notifyMemberJoined(this, hub_id, newUser.id); + + this.debug(`[_resolve_pending_invitation] Added user ${newUser.id} to hub ${hub_id}`); + } catch (err) { + // Log per-hub failure but continue processing remaining hubs + this.warn(`[_resolve_pending_invitation] Failed for hub ${hub_id}:`, err && err.message); + } + } + + // 6. Clean up all resolved pending_invitation entries for this email + + // Viral loop: every pending invitation for this address is redeemed by this + // one sign-up, so the acceptance is recorded once for all of them rather + // than per hub — invite_track_accept stamps every row still open for the + // address. Placed BEFORE pending_invitation_delete_by_email below, which + // erases the only other evidence the invitations ever existed. + // + // Never allowed to throw: an account that was created and joined its + // workspaces, then reported a failure because a metric could not be + // written, is strictly worse than an uncounted acceptance. + try { + await this.yp.await_proc("invite_track_accept", email, newUser.id); + } catch (err) { + this.warn("[butler._resolve_pending_invitation] invite tracking failed for", email, err && err.message); + } + + // The rollup counts live permission rows, so it has to be refreshed after + // the grants above — the crawl and these writers must agree. + for (const row of rows) { + try { + await this.yp.await_proc("workspace_members_set", row.hub_id); + } catch (err) { + this.warn("[butler._resolve_pending_invitation] workspace member tracking failed for", row.hub_id, err && err.message); + } + } + await this.yp.await_proc("pending_invitation_delete_by_email", email); } /** diff --git a/service/lib/resolve-pending-invitation.js b/service/lib/resolve-pending-invitation.js deleted file mode 100644 index 8cefc960..00000000 --- a/service/lib/resolve-pending-invitation.js +++ /dev/null @@ -1,360 +0,0 @@ -/** - * @license - * Copyright 2026 Thidima SA. All Rights Reserved. - * Licensed under the GNU AFFERO GENERAL PUBLIC LICENSE, Version 3. - * https://www.gnu.org/licenses/agpl-3.0.html - */ - -const { RedisStore, utils } = require("@drumee/server-essentials"); -const { isEmpty } = require("lodash"); - -// toArray from server-essentials, NOT lodash — lodash's turns an object into -// its values array, which mangles a single-row proc result (a list proc with -// exactly one row answers with the object itself). Same reason hub.js:52 and -// notify-member-joined.js pick it up from here. -const { toArray } = utils; -const { notifyMemberJoined } = require("./notify-member-joined"); - -/** - * REDEEM EVERY WORKSPACE INVITATION HELD FOR AN EMAIL ADDRESS. - * - * `yp.pending_invitation` is where an invitation waits when membership could - * not be granted at the time it was sent. Redeeming it is what actually joins - * the person to the workspace: `add_member` writes the hub's permission row AND - * calls `.join_hub`, which is what puts the workspace under their - * home root. Until that runs the invitee has NOTHING — `desk.home` lists their - * home's children, and the workspace is not among them, so it is invisible on - * their desk and stays invisible across reloads. - * - * WHY THIS IS A SHARED MODULE. Two byte-for-byte copies of this used to live in - * `signup.js` and `butler.js` — the two account-creation paths — and BOTH were - * only ever reachable at account creation. That left the case this module now - * also serves: an invitation addressed to somebody who ALREADY has an account. - * Nothing redeemed those, so they sat in the table forever. See the login hook - * in `yp.js` and `scripts/redeem-orphan-invitations.js`. - * - * NEVER THROWS PER HUB. One unreachable workspace must not cost the caller the - * others, and no caller (a sign-up, a login) may fail because of this — every - * failure is warned and swallowed. Rows that could not be redeemed are LEFT IN - * PLACE so a later attempt retries them; rows that are redeemed, expired, or - * point at a workspace that no longer exists are cleared. - * - * @param {Object} svc calling service instance — needs `yp`, `warn`, and - * `payload` when `notify` is on - * @param {String} email the invited address - * @param {Object} [opt] - * @param {String} [opt.uid] the invitee's drumate id when the caller already - * holds it (a login does) — saves a lookup - * @param {String} [opt.source] what triggered this, for the audit line - * @param {Boolean} [opt.notify] push over the socket + tell hub members - * (default true; off for offline scripts) - * @returns {Promise} { redeemed, failed, alreadyMember, hubs } - */ -async function resolvePendingInvitations(svc, email, opt = {}) { - const { source = "signup", notify = true } = opt; - const out = { redeemed: 0, failed: 0, alreadyMember: 0, hubs: [] }; - if (!svc || !svc.yp || isEmpty(email)) return out; - - const warn = (...a) => svc.warn && svc.warn("[pending-invitation]", ...a); - - // The lookup FIRST, and it is the whole cost on the common path: almost every - // caller has no pending row, and this returns empty without touching anything - // else. That is what makes it cheap enough to run on every login. - let rows; - try { - rows = toArray(await svc.yp.await_proc("pending_invitation_get_by_email", email)); - } catch (e) { - warn("lookup failed for", email, e && e.message); - return out; - } - if (isEmpty(rows)) return out; - - let uid = opt.uid || null; - if (!uid) { - try { - let user = await svc.yp.await_proc("drumate_exists", email); - if (Array.isArray(user)) user = user[0]; - uid = user && user.id; - } catch (e) { - warn("drumate_exists failed for", email, e && e.message); - return out; - } - } - // No account yet — the invitation is not orphaned, it is simply still - // waiting. Leave every row where it is: account creation redeems them. - if (!uid) return out; - - const { writeAudit } = require("../private/_audit"); - const now = Math.floor(Date.now() / 1000); - // Rows we are done with, redeemed or not — see the delete at the bottom. - let unresolved = 0; - - for (const row of rows) { - const hub_id = row && row.hub_id; - if (!hub_id) continue; - try { - // ⚠️ UNITS. `pending_invitation.expiry_time` holds an ABSOLUTE unix - // timestamp (yp_add_pending_invitation converts the caller's hours with - // TIMESTAMPADD before storing), while `add_member` and `permission_grant` - // both take HOURS FROM NOW and do that same conversion themselves. - // Handing the stored value straight to them — which both old copies of - // this code did — asks for a grant expiring in ~1.8 billion hours. It - // errs open so nothing was ever reported, but it is not what the inviter - // set, and it is why this converts back. - const stored = ~~row.expiry_time; - if (stored && stored <= now) { - // Expired before it was ever redeemed. Nothing to grant; the row is - // cleared with the rest so it stops being retried. - continue; - } - const hours = stored ? Math.max(1, Math.ceil((stored - now) / 3600)) : 0; - - const db_name = await svc.yp.await_func("get_db_name", hub_id); - if (!db_name) { - // The workspace is gone. Stale row, not a failure — clearing it is the - // repair, and keeping it would mean retrying a dead hub on every login. - warn("no db_name for hub", hub_id, "— dropping stale invitation"); - continue; - } - - // 🚨 DO NOT WALK OVER A MEMBERSHIP THAT ALREADY WORKS. - // - // A pending row is deleted only when it is redeemed, so a STALE one can - // sit beside a membership that was later granted properly through - // `hub.invite` — and this runs on every login. Re-granting from the stale - // row would write ITS privilege over the live one (an admin re-invited at - // 15 would be demoted back to the 4 the old invitation carried), and - // `add_member` also re-runs `join_hub`, whose REPLACE re-derives the - // home-root row's filename through `unique_filename` and resets its rank - // — renaming and reshuffling a workspace the user has had for months. - // - // So: already in, and the workspace already sitting under their home root - // → nothing to repair, just clear the row. `state` is read rather than - // assumed because "has permission" and "can see it" are two different - // writes inside add_member, and a half-applied one is exactly what this - // module exists to finish. - const state = await _membership(svc, db_name, hub_id, uid, warn); - if (state.permission > 0 && state.visible) { - out.alreadyMember++; - continue; - } - // Never downgrade on the repair path either: a live grant that is higher - // than the invitation's wins. - const permission = Math.max(~~row.permission, state.permission); - - const r = await svc.yp.await_proc( - `${db_name}.add_member`, uid, permission, hours - ); - // add_member answers with a db_name-carrying row only when it actually - // wrote the membership; anything else means it bailed (an unknown member, - // or the hub inviting itself) and permission_grant below would create a - // grant with no membership behind it. - if (!r || !r.db_name) { - warn("add_member wrote nothing for", uid, "on hub", hub_id); - unresolved++; - out.failed++; - continue; - } - - await svc.yp.await_proc( - `${db_name}.permission_grant`, - "*", uid, hours, permission, "system", - `Resolved from pending_invitation (${source})` - ); - - // Chat upload, the same grant `hub._grantMembership` makes. Neither old - // copy of this code made it, so a member who arrived through a pending - // invitation could not attach a file in the workspace chat. - try { - const mfs_home = await svc.yp.await_proc(`${db_name}.mfs_home`); - if (mfs_home && mfs_home.chat_upload_id) { - await svc.yp.await_proc( - `${db_name}.permission_grant`, - mfs_home.chat_upload_id, uid, 0, 4, - "no_traversal", "chat upload permission" - ); - } - } catch (e) { - warn("chat upload grant failed for hub", hub_id, e && e.message); - } - - await writeAudit(svc, { - db: db_name, - uid, - action: "invite_accepted", - category: "member", - notify_to: "admin", - entity_id: hub_id, - log: `Invite accepted — ${email} joined the workspace (${source})`, - }); - - // The notification row the invitee reads in the bell. Written by - // `_grantMembership` on the immediate-grant path and by nothing at all on - // this one, so a redeemed invitation used to arrive with no trace: the - // workspace simply appeared. - // - // AUTHORED BY THE WORKSPACE OWNER, not by whoever sent the invitation: - // `pending_invitation` stores only (hub_id, email, permission, expiry), - // so the inviter is not recoverable here. The owner is the truthful - // stand-in — the row must name SOMEBODY, and naming the invitee (the only - // id in hand) would render as "you invited yourself". Skipped rather than - // faked when even the owner cannot be read. - try { - // A plain read, not `get_hub`: that proc is keyed by vhost and takes a - // visitor as its second argument, so it answers nothing useful here. - let info = typeof svc.yp.await_query !== "function" ? null : await svc.yp.await_query( - "SELECT owner_id, name FROM hub WHERE id=?", hub_id - ); - if (Array.isArray(info)) info = info[0]; - const author = info && info.owner_id; - if (author) { - await svc.yp.await_proc( - "contact_log_activity", author, uid, "hub_invite_received", - { - hub_id, - hub_name: (info && info.name) || null, - permission, - } - ); - } - } catch (e) { - warn("activity log failed for hub", hub_id, e && e.message); - } - - out.redeemed++; - out.hubs.push(hub_id); - - if (notify) await _push(svc, db_name, hub_id, uid, warn); - } catch (err) { - unresolved++; - out.failed++; - warn("failed for hub", hub_id, err && err.message); - } - } - - if (!out.redeemed) { - // Nothing was granted, which is not the same as nothing being resolved: the - // rows may all have been expired, stale, or already honoured. Those are - // finished with. A FAILURE is not — and there is no per-row delete, so the - // blanket one is only safe when no row still wants retrying. - if (!unresolved) await _clear(svc, email, warn); - return out; - } - - // Viral loop: one acceptance stamps every row still open for the address, so - // it runs once rather than per hub — and BEFORE the delete below, which - // erases the only other evidence the invitations existed. Never throws: an - // uncounted acceptance beats a join that reports failure. - try { - await svc.yp.await_proc("invite_track_accept", email, uid); - } catch (e) { - warn("invite tracking failed for", email, e && e.message); - } - - // The rollup counts live permission rows, so it is refreshed after the grants - // — the crawl and these writers have to agree. - for (const hub_id of out.hubs) { - try { - await svc.yp.await_proc("workspace_members_set", hub_id); - } catch (e) { - warn("member tracking failed for", hub_id, e && e.message); - } - } - - if (!unresolved) await _clear(svc, email, warn); - return out; -} - -/** - * What the invitee already has in this workspace. - * - * TWO SEPARATE FACTS, because `add_member` writes them into two different - * databases and a half-applied one is exactly what this module repairs: - * - * permission — the membership row in the HUB's permission table - * (`resource_id = '*'`), which is what `user_permission` reads - * and what `mfs_show_node_by` filters `privilege > 0` on. - * visible — the hub's row under the invitee's own home root, written by - * `join_hub`. Without it `desk.home` has nothing to list, so the - * workspace does not appear however good the permission is. - * - * Read directly rather than through `user_permission`: that function walks - * parents and falls back to the anonymous grants, so it answers > 0 for a - * public workspace nobody was ever added to. Here the question is strictly - * "does this person hold a membership row". - * - * A read that fails answers "nothing" — which sends the caller down the grant - * path. That is the safe direction: the same hub DB is about to be written - * anyway, so a failure here surfaces there as a failure rather than as a - * silently skipped repair. - */ -async function _membership(svc, db_name, hub_id, uid, warn) { - const state = { permission: 0, visible: false }; - if (typeof svc.yp.await_query !== "function") return state; - try { - let row = await svc.yp.await_query( - `SELECT permission FROM \`${db_name}\`.permission - WHERE resource_id='*' AND entity_id=? LIMIT 1`, uid - ); - if (Array.isArray(row)) row = row[0]; - state.permission = ~~(row && row.permission); - } catch (e) { - warn("membership read failed for hub", hub_id, e && e.message); - return state; - } - if (!state.permission) return state; - try { - const own_db = await svc.yp.await_func("get_db_name", uid); - if (!own_db) return state; - let row = await svc.yp.await_query( - `SELECT id FROM \`${own_db}\`.media WHERE id=? LIMIT 1`, hub_id - ); - if (Array.isArray(row)) row = row[0]; - state.visible = !!(row && row.id); - } catch (e) { - warn("home-root read failed for hub", hub_id, e && e.message); - } - return state; -} - -/** - * Tell the invitee's own live session, then the workspace's other members. - * - * `hub.invite_received` is what the desk listens for — the sidebar restarts its - * list on it and the topbar switcher resyncs (ui-team desk/index.js - * `_onWorkspaceWsEvent`, desk/workspace-list `handleWsEvent`) — so an invitation - * redeemed while the recipient is looking at the desk lands without a reload. - */ -async function _push(svc, db_name, hub_id, uid, warn) { - try { - if (typeof svc.payload !== "function") return; - const hub = await svc.yp.await_proc(`${db_name}.mfs_access_node`, uid, hub_id); - if (hub) { - hub.ownpath = "/"; - hub.hub_id = hub.actual_hub_id; - hub.db_name = hub.actual_db; - const sockets = await svc.yp.await_proc("user_sockets", uid); - await RedisStore.sendData( - svc.payload(hub, { service: "hub.invite_received" }), sockets - ); - await RedisStore.sendData( - svc.payload(hub, { service: "hub.add_contributors" }), sockets - ); - } - } catch (e) { - warn("ws notify failed for hub", hub_id, e && e.message); - } - // Any admin with the member matrix open refetches instead of showing a list - // that is missing the person who just joined. - await notifyMemberJoined(svc, hub_id, uid); -} - -async function _clear(svc, email, warn) { - try { - await svc.yp.await_proc("pending_invitation_delete_by_email", email); - } catch (e) { - warn("cleanup failed for", email, e && e.message); - } -} - -module.exports = { resolvePendingInvitations }; diff --git a/service/private/hub.js b/service/private/hub.js index 54a54e2e..c19a8801 100644 --- a/service/private/hub.js +++ b/service/private/hub.js @@ -738,6 +738,7 @@ class __private_hub extends Hub { this.output.data([]); return; } + let { domain_id } = this.user.toJSON(); let db_name = await this.yp.await_func("get_db_name", this.uid); if (!db_name) { this.warn("[hub] add_contributors: no contact db for hub", this.uid); @@ -747,42 +748,29 @@ class __private_hub extends Hub { let proc = `${db_name}.my_contact_exists`; for (let entity of users) { try { - const contact = await this.yp.await_proc(proc, 'entity', entity, '', ''); - - // WHO GETS ADDED ON THE SPOT: anybody who already holds a Drumee - // account. Full stop. - // - // 🚨 THIS USED TO BE GATED ON THE INVITER. Membership was granted only - // when the address was an ACTIVE CONTACT of the caller, or when the - // account lived in the caller's own `domain_id`; everything else was - // written to `yp.pending_invitation` instead. Both tests describe the - // INVITER's address book and the INVITER's organisation — neither says - // anything about whether the invitee can hold a membership, and - // `hub.invite` (the popup, the folder access panel, the restricted - // permission panel) has never applied either of them. - // - // The domain half was the damaging one, because a pending row is only - // ever redeemed at ACCOUNT CREATION (signup / butler). Send it to an - // address that already has an account and nothing redeems it, ever: no - // add_member, so no `join_hub`, so no workspace under the invitee's - // home root, so `desk.home` never lists it and the workspace is - // invisible on their desk across reloads. Free accounts all sit in - // domain 1 until `org_provision` moves them, so ANY invitation from a - // provisioned org to a free user landed there — reported as "I was - // invited to a workspace and I cannot see it". - // - // Authorisation is the hub's permission table, which add_member and - // permission_grant below write. Domain equality is not, and was not, - // an access rule anywhere else in the stack. - let uid = null; - if (!isEmpty(contact) && contact.status == "active") uid = contact.uid; - // AND FALL THROUGH WHEN THE CONTACT CARRIES NO UID. An address-book row - // can be `active` with an empty `uid` (it is filled in when the contact - // is matched to an account), and the old code pushed that empty value - // into `members` — where add_member quietly bailed and the person was - // never added. Asking `drumate_exists` is the answer to the same - // question, one query later. - if (!uid) { + let contact = await this.yp.await_proc(proc, 'entity', entity, '', ''); + if (!isEmpty(contact)) { + if (contact.status == "active") { + members.push(contact.uid); + } else { + await this.yp.await_proc( + "yp_add_pending_invitation", + this.hub.get(Attr.id), + expiry, + privilege, + entity + ); + await writeAudit(this, { + db: this.hub.get(Attr.db_name), + uid: this.uid, + action: 'invite_sent', + category: 'member', + notify_to: 'admin', + entity_id: this.hub.get(Attr.id), + log: `Invite sent to ${entity} for workspace '${hubname}'`, + }); + } + } else { let drumate = null; try { drumate = await this.yp.await_proc("drumate_exists", entity); @@ -790,55 +778,53 @@ class __private_hub extends Hub { } catch (e) { this.warn("[hub] add_contributors: drumate_exists for entity", entity, e); } - if (drumate && drumate.id) uid = drumate.id; - } - - if (uid) { - members.push(uid); - continue; - } - - // NO ACCOUNT YET. Register the invitation so account creation grants it - // (signup's create_account -> _resolve_pending_invitation), and send - // the invitation email. - await this.yp.await_proc( - "yp_add_pending_invitation", - this.hub.get(Attr.id), - expiry, - privilege, - entity - ); - await writeAudit(this, { - db: this.hub.get(Attr.db_name), - uid: this.uid, - action: 'invite_sent', - category: 'member', - notify_to: 'admin', - entity_id: this.hub.get(Attr.id), - log: `Invite sent to ${entity} for workspace '${hubname}'`, - }); - const isEmail = typeof entity === "string" && entity.indexOf("@") !== -1; - if (isEmail) { - try { - const ContactPrivate = require("./contact"); - const contactSvc = new ContactPrivate({ - session: this.session, - permission: this.permission || { scope: "hub" } + const sameDomain = drumate && drumate.domain_id != null && domain_id === drumate.domain_id; + if (sameDomain) { + members.push(drumate.id); + } else { + // Not a contact: register as pending invitation and send contact invitation. + // Two cases: (1) Email already has Drumee account → contact.invite sends in-app mail. + // (2) Email not in system → contact.invite sends signup/invite email with token. + await this.yp.await_proc( + "yp_add_pending_invitation", + this.hub.get(Attr.id), + expiry, + privilege, + entity + ); + await writeAudit(this, { + db: this.hub.get(Attr.db_name), + uid: this.uid, + action: 'invite_sent', + category: 'member', + notify_to: 'admin', + entity_id: this.hub.get(Attr.id), + log: `Invite sent to ${entity} for workspace '${hubname}'`, }); - contactSvc.db = { - await_proc: (proc, ...args) => this.yp.await_proc(`${db_name}.${proc}`, ...args), - end: () => Promise.resolve() - }; - const origEmail = this.input.use(Attr.email) ?? this.input.get(Attr.email); - const origMessage = this.input.use(Attr.message); - this.input.set(Attr.email, entity); - this.input.set(Attr.message, message); - this.input.set("_contact_db_name", db_name); - await contactSvc.invite(); - if (origEmail !== undefined) this.input.set(Attr.email, origEmail); - if (origMessage !== undefined) this.input.set(Attr.message, origMessage); - } catch (err) { - this.warn("[hub] add_contributors: send invitation failed for entity", entity, err); + const isEmail = typeof entity === "string" && entity.indexOf("@") !== -1; + if (isEmail) { + try { + const ContactPrivate = require("./contact"); + const contactSvc = new ContactPrivate({ + session: this.session, + permission: this.permission || { scope: "hub" } + }); + contactSvc.db = { + await_proc: (proc, ...args) => this.yp.await_proc(`${db_name}.${proc}`, ...args), + end: () => Promise.resolve() + }; + const origEmail = this.input.use(Attr.email) ?? this.input.get(Attr.email); + const origMessage = this.input.use(Attr.message); + this.input.set(Attr.email, entity); + this.input.set(Attr.message, message); + this.input.set("_contact_db_name", db_name); + await contactSvc.invite(); + if (origEmail !== undefined) this.input.set(Attr.email, origEmail); + if (origMessage !== undefined) this.input.set(Attr.message, origMessage); + } catch (err) { + this.warn("[hub] add_contributors: send invitation failed for entity", entity, err); + } + } } } } catch (err) { @@ -1939,6 +1925,7 @@ class __private_hub extends Hub { const username = this.user.get('fullname'); const lang = this.user.language() || this.input.app_language(); + const { domain_id } = this.user.toJSON(); const expiry = 0; // No expiry option in UI // Inviter's contact DB — used for my_contact_exists lookups @@ -1986,21 +1973,38 @@ class __private_hub extends Hub { // Resolve each user entity for (const entity of users) { try { - // 1. Resolve the entity to an ACCOUNT. Anybody who already holds one - // is granted membership on the spot — see the long note in - // add_contributors: the "active contact of the inviter, or same - // domain_id" test that used to stand here describes the INVITER, - // and a pending row addressed to an existing account is redeemed - // by nothing, so the workspace never appeared on their desk. + // 1. Check inviter's contact list const contact = await this.yp.await_proc(contact_proc, 'entity', entity, '', ''); - let uid = null; - let uidEmail = null; - if (!isEmpty(contact) && contact.status === 'active' && contact.uid) { - uid = contact.uid; - uidEmail = contact.email || asEmail(entity); + if (!isEmpty(contact)) { + if (contact.status === 'active') { + // Known active contact → add immediately + members.push(contact.uid); + memberEmail.set(contact.uid, contact.email || asEmail(entity)); + } else { + // Pending contact → store for deferred grant + await this.yp.await_proc( + 'yp_add_pending_invitation', + hub_id, expiry, privilege, entity + ); + await writeAudit(this, { + db: hub_db, + uid: this.uid, + action: 'invite_sent', + category: 'member', + notify_to: 'admin', + entity_id: hub_id, + log: `Invite sent to ${entity} for workspace '${hubname}'`, + }); + await this._trackInviteSent({ + hub_id, + email: asEmail(entity), + had_account: false, + source: "invite_with_roles", + }); + } } else { - // Including an `active` contact with no uid — see add_contributors. + // Not in contact list — check if user exists in Drumee let drumate = null; try { drumate = await this.yp.await_proc('drumate_exists', entity); @@ -2008,67 +2012,67 @@ class __private_hub extends Hub { } catch (e) { this.warn('[hub] invite_with_roles: drumate_exists failed for', entity, e); } - if (drumate && drumate.id) { - uid = drumate.id; - uidEmail = drumate.email || asEmail(entity); - } - } - - if (uid) { - members.push(uid); - memberEmail.set(uid, uidEmail); - continue; - } - // 2. No account yet → pending + invite email. Account creation - // redeems the row (signup -> _resolve_pending_invitation). - await this.yp.await_proc( - 'yp_add_pending_invitation', - hub_id, expiry, privilege, entity - ); - await writeAudit(this, { - db: hub_db, - uid: this.uid, - action: 'invite_sent', - category: 'member', - notify_to: 'admin', - entity_id: hub_id, - log: `Invite sent to ${entity} for workspace '${hubname}'`, - }); - await this._trackInviteSent({ - hub_id, - email: asEmail(entity), - had_account: false, - source: "invite_with_roles", - }); - - // Only send email if entity looks like an email address - const isEmail = typeof entity === 'string' && entity.indexOf('@') !== -1; - if (isEmail) { - try { - const ContactPrivate = require('./contact'); - const contactSvc = new ContactPrivate({ - session: this.session, - permission: this.permission || { scope: 'hub' }, - }); - contactSvc.db = { - await_proc: (proc, ...args) => - this.yp.await_proc(`${contact_db}.${proc}`, ...args), - end: () => Promise.resolve(), - }; - const origEmail = this.input.use(Attr.email); - const origMessage = this.input.use(Attr.message); - this.input.set(Attr.email, entity); - this.input.set(Attr.message, msg); - this.input.set('_contact_db_name', contact_db); - await contactSvc.invite(); - if (origEmail !== undefined) this.input.set(Attr.email, origEmail); - if (origMessage !== undefined) this.input.set(Attr.message, origMessage); - } catch (err) { - this.warn( - '[hub] invite_with_roles: send invitation failed for', - entity, err + const sameDomain = + drumate && + drumate.domain_id != null && + domain_id === drumate.domain_id; + + if (sameDomain) { + // Exists on same domain → add immediately + members.push(drumate.id); + memberEmail.set(drumate.id, drumate.email || asEmail(entity)); + } else { + // Unknown user or different domain → pending + invite email + await this.yp.await_proc( + 'yp_add_pending_invitation', + hub_id, expiry, privilege, entity ); + await writeAudit(this, { + db: hub_db, + uid: this.uid, + action: 'invite_sent', + category: 'member', + notify_to: 'admin', + entity_id: hub_id, + log: `Invite sent to ${entity} for workspace '${hubname}'`, + }); + await this._trackInviteSent({ + hub_id, + email: asEmail(entity), + had_account: false, + source: "invite_with_roles", + }); + + // Only send email if entity looks like an email address + const isEmail = typeof entity === 'string' && entity.indexOf('@') !== -1; + if (isEmail) { + try { + const ContactPrivate = require('./contact'); + const contactSvc = new ContactPrivate({ + session: this.session, + permission: this.permission || { scope: 'hub' }, + }); + contactSvc.db = { + await_proc: (proc, ...args) => + this.yp.await_proc(`${contact_db}.${proc}`, ...args), + end: () => Promise.resolve(), + }; + const origEmail = this.input.use(Attr.email); + const origMessage = this.input.use(Attr.message); + this.input.set(Attr.email, entity); + this.input.set(Attr.message, msg); + this.input.set('_contact_db_name', contact_db); + await contactSvc.invite(); + if (origEmail !== undefined) this.input.set(Attr.email, origEmail); + if (origMessage !== undefined) this.input.set(Attr.message, origMessage); + } catch (err) { + this.warn( + '[hub] invite_with_roles: send invitation failed for', + entity, err + ); + } + } } } } catch (err) { @@ -2140,17 +2144,7 @@ class __private_hub extends Hub { hub.hub_id = hub.actual_hub_id; hub.db_name = hub.actual_db; const sockets = await this.yp.await_proc('user_sockets', recipient.id); - // NAMED, like invite()'s and add_contributors' own pushes. An unnamed - // payload arrives at the desk with `options.service` undefined, and - // every consumer keys off exactly that: the dispatcher's default branch - // re-emits it as `ws:event`, the topbar switcher drops it on - // `if (!service) return`, and the sidebar's handler falls through to - // super. So a member added here was granted correctly and then had to - // reload to find out — the same invisible-workspace symptom, one layer - // up from the grant. - await RedisStore.sendData( - this.payload(hub, { service: "hub.invite_received" }), sockets - ); + await RedisStore.sendData(this.payload(hub), sockets); } // One broadcast per hub is enough — the matrix refetches the whole diff --git a/service/scripts/redeem-orphan-invitations.js b/service/scripts/redeem-orphan-invitations.js deleted file mode 100644 index e4c2641e..00000000 --- a/service/scripts/redeem-orphan-invitations.js +++ /dev/null @@ -1,133 +0,0 @@ -/** - * @license - * Copyright 2026 Thidima SA. All Rights Reserved. - * Licensed under the GNU AFFERO GENERAL PUBLIC LICENSE, Version 3. - * https://www.gnu.org/licenses/agpl-3.0.html - */ - -/** - * REPAIR WORKSPACE INVITATIONS THAT WERE NEVER GRANTED. - * - * An orphaned invitation is a `yp.pending_invitation` row whose address ALREADY - * BELONGS TO A DRUMEE ACCOUNT. Those rows are only ever redeemed at account - * creation, so for an existing account nothing redeemed them: no `add_member`, - * therefore no `join_hub`, therefore no workspace under the invitee's home root - * and nothing for `desk.home` to return. The workspace is invisible on their - * desk and stays invisible across reloads. - * - * `hub.add_contributors` / `hub.invite_with_roles` no longer create such rows - * (they grant to any existing account, whatever domain it is in), and - * `yp.login` redeems leftovers on the invitee's next sign-in. This script is - * the immediate, complete pass — it does not wait for people to log in. - * - * SAFE TO RE-RUN, and safe next to memberships that already work. Every row is - * checked before anything is written: somebody already holding the membership - * AND already carrying the workspace under their home root is left untouched - * (their stale invitation is simply cleared), and a repair never lowers a - * privilege that was raised since the invitation was sent. Rows that fail are - * kept for the next run. - * - * DRY RUN BY DEFAULT — it prints what it would do and writes nothing. - * - * Usage: - * node service/scripts/redeem-orphan-invitations.js # report only - * node service/scripts/redeem-orphan-invitations.js --apply # do it - * node service/scripts/redeem-orphan-invitations.js --apply --email=a@b.c - */ - -const { Mariadb } = require("@drumee/server-essentials"); -const { resolvePendingInvitations } = require("../lib/resolve-pending-invitation"); - -const argv = process.argv.slice(2); -const APPLY = argv.includes("--apply"); -const ONE = (argv.find((a) => a.startsWith("--email=")) || "").split("=")[1] || null; - -/** - * The shape `resolvePendingInvitations` needs. No `payload`, so it skips the - * websocket push — this runs outside any request and the recipients are - * overwhelmingly offline; their next desk load reads `desk.home` fresh anyway. - */ -function shim(yp) { - return { - yp, - warn: (...a) => console.warn(" !", ...a), - debug: () => {}, - }; -} - -async function main() { - const stamp = new Date().toISOString(); - console.log(`[${stamp}] orphaned workspace invitations — ${APPLY ? "APPLY" : "DRY RUN"}`); - - const yp = new Mariadb({ name: "yp" }); - try { - // Orphaned = the address already has an account. `drumate.email` and - // `pending_invitation.email` are both utf8mb4_general_ci, so the join is - // case-insensitive, which is what the redeemer's own lookup does too. - const sql = ` - SELECT p.email, d.id AS uid, COUNT(*) AS pending, MIN(p.created_at) AS oldest - FROM pending_invitation p - INNER JOIN drumate d ON d.email = p.email - ${ONE ? "WHERE p.email = ?" : ""} - GROUP BY p.email, d.id - ORDER BY oldest ASC - `; - const rows = (ONE ? await yp.await_query(sql, ONE) : await yp.await_query(sql)) || []; - // A one-row result comes back as the object itself, and a no-row one can - // come back as `{}` — filter on a field every real row has rather than on - // length, or an empty answer reads as one account with `undefined` rows. - const list = (Array.isArray(rows) ? rows : [rows]).filter((r) => r && r.email); - - if (!list.length) { - console.log("Nothing to repair."); - await yp.end(); - return; - } - - console.log(`${list.length} account(s) holding un-granted invitations:\n`); - let redeemed = 0; - let failed = 0; - // Rows whose workspace the person is already in — a stale invitation left - // behind by a later, successful invite. Cleared, never re-granted. - let skipped = 0; - - for (const row of list) { - const age = row.oldest - ? `${Math.floor((Date.now() / 1000 - row.oldest) / 86400)}d` - : "?"; - console.log(` ${row.email} (${row.pending} invitation(s), oldest ${age})`); - if (!APPLY) continue; - - const res = await resolvePendingInvitations(shim(yp), row.email, { - uid: row.uid, - source: "repair-script", - notify: false, - }); - redeemed += res.redeemed; - failed += res.failed; - skipped += res.alreadyMember; - console.log( - ` → redeemed ${res.redeemed}, already a member ${res.alreadyMember}` - + `, failed ${res.failed}` - ); - } - - console.log(""); - if (!APPLY) { - console.log("Dry run — nothing written. Re-run with --apply."); - } else { - console.log( - `Done: ${redeemed} membership(s) granted, ${skipped} already in place` - + `, ${failed} failure(s).` - ); - if (failed) console.log("Failed rows were LEFT IN PLACE — re-run to retry them."); - } - await yp.end(); - } catch (e) { - console.error("FAILED:", e && e.message); - try { await yp.end(); } catch (_) {} - process.exitCode = 1; - } -} - -main(); diff --git a/service/signup.js b/service/signup.js index 855133b4..f3ccff08 100644 --- a/service/signup.js +++ b/service/signup.js @@ -16,10 +16,11 @@ */ const { - Attr, Messenger, Cache, sysEnv, uniqueId + Attr, Messenger, Cache, sysEnv, uniqueId, RedisStore } = require("@drumee/server-essentials"); const { resolve } = require('path'); const { isEmpty } = require("lodash"); +const { notifyMemberJoined } = require("./lib/notify-member-joined"); const { Mfs } = require("@drumee/server-core"); class __signup extends Mfs { @@ -146,15 +147,92 @@ class __signup extends Mfs { /** * Resolve pending hub invitations for a newly created user. - * - * The body of this used to live here, duplicated byte-for-byte in butler.js, - * and reachable from account creation ONLY. Both copies now defer to - * `service/lib/resolve-pending-invitation`, which the login path and the - * repair script share — see that module for why that matters. + * Mirrors butler._resolve_pending_invitation. */ async _resolve_pending_invitation(email) { - const { resolvePendingInvitations } = require("./lib/resolve-pending-invitation"); - return resolvePendingInvitations(this, email, { source: "signup" }); + const { toArray } = require("@drumee/server-essentials").utils; + + const newUser = await this.yp.await_proc("drumate_exists", email); + if (isEmpty(newUser) || !newUser.id) return; + + const pending = await this.yp.await_proc("pending_invitation_get_by_email", email); + const rows = toArray(pending); + if (isEmpty(rows)) return; + + for (const row of rows) { + const { hub_id, permission, expiry_time } = row; + try { + const db_name = await this.yp.await_func("get_db_name", hub_id); + if (!db_name) continue; + await this.yp.await_proc(`${db_name}.add_member`, newUser.id, permission, expiry_time); + await this.yp.await_proc( + `${db_name}.permission_grant`, + '*', newUser.id, expiry_time, permission, 'system', 'Resolved from pending_invitation on signup' + ); + // Audit: invite redeemed on signup — never let a failure break the join. + try { + const { writeAudit } = require("./private/_audit"); + await writeAudit(this, { + db: db_name, + uid: newUser.id, + action: 'invite_accepted', + category: 'member', + notify_to: 'admin', + entity_id: hub_id, + log: `Invite accepted — ${email} joined the workspace on signup`, + }); + } catch (e) { /* audit only */ } + + // Notify the (now online) user so the desk sidebar can pick up the + // new workspace, mirroring hub.invite()'s notification for drumates. + try { + const hub = await this.yp.await_proc(`${db_name}.mfs_access_node`, newUser.id, hub_id); + if (hub) { + hub.ownpath = '/'; + hub.hub_id = hub.actual_hub_id; + hub.db_name = hub.actual_db; + const sockets = await this.yp.await_proc('user_sockets', newUser.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(`[signup._resolve_pending_invitation] WS notify failed for hub ${hub_id}:`, err && err.message); + } + + // Tell online members a member joined so an open permission matrix + // refreshes (mirrors butler.js _resolve_pending_invitation). + await notifyMemberJoined(this, hub_id, newUser.id); + } catch (err) { + this.warn(`[signup._resolve_pending_invitation] Failed for hub ${hub_id}:`, err && err.message); + } + } + + + // Viral loop: every pending invitation for this address is redeemed by this + // one sign-up, so the acceptance is recorded once for all of them rather + // than per hub — invite_track_accept stamps every row still open for the + // address. Placed BEFORE pending_invitation_delete_by_email below, which + // erases the only other evidence the invitations ever existed. + // + // Never allowed to throw: an account that was created and joined its + // workspaces, then reported a failure because a metric could not be + // written, is strictly worse than an uncounted acceptance. + try { + await this.yp.await_proc("invite_track_accept", email, newUser.id); + } catch (err) { + this.warn("[signup._resolve_pending_invitation] invite tracking failed for", email, err && err.message); + } + + // The rollup counts live permission rows, so it has to be refreshed after + // the grants above — the crawl and these writers must agree. + for (const row of rows) { + try { + await this.yp.await_proc("workspace_members_set", row.hub_id); + } catch (err) { + this.warn("[signup._resolve_pending_invitation] workspace member tracking failed for", row.hub_id, err && err.message); + } + } + await this.yp.await_proc("pending_invitation_delete_by_email", email); } } diff --git a/service/yp.js b/service/yp.js index ea6bcc14..5be5dc50 100644 --- a/service/yp.js +++ b/service/yp.js @@ -313,67 +313,9 @@ class __yp extends Entity { } await this._stampPasswordBacked(r, vars); - await this._redeemPendingInvitations(r, vars); this.output.data(r); } - /** - * SELF-HEAL AN INVITATION THAT WAS NEVER GRANTED. - * - * `yp.pending_invitation` holds invitations that could not be granted when - * they were sent, and until now the only thing that ever redeemed one was - * ACCOUNT CREATION. An invitation addressed to somebody who already had an - * account therefore sat there forever: no membership, no `join_hub`, so no - * workspace under their home root and nothing for `desk.home` to list — the - * workspace was simply invisible to them, reload after reload. - * - * The source of those rows is fixed (hub.add_contributors / invite_with_roles - * no longer refuse to grant across domains), so this is the net underneath: - * anyone already carrying an orphaned row is repaired the next time they sign - * in, with no support ticket and nothing for them to click. - * - * ONE INDEXED READ on the common path. `pending_invitation` is keyed by - * (hub_id, email) and almost every login finds nothing, in which case the - * helper returns before touching anything else. The uid is passed in so it - * does not have to look the account up again. - * - * NEVER FAILS A LOGIN. The session is already established by the time this - * runs; a repair that cannot complete is warned and retried on the next - * sign-in. - */ - async _redeemPendingInvitations(r, vars = {}) { - try { - if (!r || r.status !== "ok" || !r.user || !r.user.id) return; - const profile = isString(r.user.profile) - ? this.session.parseJSON(r.user.profile) - : (r.user.profile || {}); - let email = r.user.email - || profile.email - || (vars.uid && vars.uid.isEmail && vars.uid.isEmail() ? vars.uid : null); - // `pending_invitation` is keyed by ADDRESS, so without one there is - // nothing to look up. get_user's shape varies by path (the 2FA leg builds - // `r` from it directly), so resolve it rather than give up. - if (!email) { - let row = await this.yp.await_proc("drumate_exists", r.user.id); - if (isArray(row)) row = row[0]; - email = row && row.email; - } - if (!email) return; - const { resolvePendingInvitations } = require("./lib/resolve-pending-invitation"); - const res = await resolvePendingInvitations(this, email, { - uid: r.user.id, - source: "login", - }); - if (res.redeemed) { - this.debug( - "[login] redeemed", res.redeemed, "orphaned workspace invitation(s) for", email - ); - } - } catch (e) { - this.warn("login: pending invitation redemption failed:", e && e.message); - } - } - /** * A successful PASSWORD login is ground truth that the account holds a * real password. Legacy and OAuth-linked profiles may lack the @@ -460,9 +402,6 @@ class __yp extends Entity { } await logConnection(this, uid); const user = await this.yp.await_proc('get_user', uid); - // Same repair as the password path — 2FA accounts complete their sign-in - // here and never pass through login()'s tail. See _redeemPendingInvitations. - await this._redeemPendingInvitations({ status: "ok", user }); this.output.data(user); } diff --git a/test/pending-invitation-redeem.test.js b/test/pending-invitation-redeem.test.js deleted file mode 100644 index dd2d74d4..00000000 --- a/test/pending-invitation-redeem.test.js +++ /dev/null @@ -1,250 +0,0 @@ -// Redeeming a workspace invitation that was never granted. -// -// THE REPORT: "I skipped creating a workspace during onboarding, a colleague -// invited me to theirs, and I cannot see it." The invitation had been written -// to yp.pending_invitation instead of being granted (hub.add_contributors -// refused to grant across domains, and a free account sits in domain 1 until -// org_provision moves it). Nothing redeems a pending row for an account that -// ALREADY EXISTS — both old redeemers ran at account creation only — so there -// was no add_member, therefore no join_hub, therefore no workspace under the -// invitee's home root and nothing for desk.home to list. Invisible, reload -// after reload. -// -// These lock the redeemer's contract: who gets granted, what is cleared, and -// what is deliberately left behind to be retried. -const assert = require('node:assert/strict'); -const test = require('node:test'); - -global.myDrumee = { arch: 'pod', useEmail: 0 }; -global.verbosity = 0; -global.debug = {}; - -const HOUR = 3600; -const NOW = () => Math.floor(Date.now() / 1000); - -// The lib destructures RedisStore once at load, so patching the method on that -// same object is what lets a test see the push without a live redis. -const essentials = require('@drumee/server-essentials'); -const sent = []; -essentials.RedisStore.sendData = async (payload, dest) => { sent.push({ payload, dest }); }; - -const { resolvePendingInvitations } = require('../service/lib/resolve-pending-invitation'); - -/** - * A yp double that records every call and answers the happy path by default. - * `answers` overrides one proc by name; `null` from an override means "no row". - */ -function fakeSvc(pending, answers = {}) { - const calls = []; - const pick = (name, args) => { - if (Object.prototype.hasOwnProperty.call(answers, name)) { - const a = answers[name]; - return typeof a === 'function' ? a(...args) : a; - } - return undefined; - }; - const yp = { - async await_proc(name, ...args) { - calls.push([name, ...args]); - const o = pick(name, args); - if (o !== undefined) return o; - if (name === 'pending_invitation_get_by_email') return pending; - if (/\.add_member$/.test(name)) return { id: 'u_invitee', db_name: 'a_invitee' }; - if (/\.mfs_home$/.test(name)) return { chat_upload_id: 'chat1' }; - if (/\.mfs_access_node$/.test(name)) return { actual_hub_id: args[1], actual_db: 'f_hub' }; - if (name === 'user_sockets') return [{ id: 's1' }]; - if (name === 'entity_sockets') return [{ id: 's2' }]; - return {}; - }, - async await_func(name, ...args) { - calls.push([name, ...args]); - const o = pick(name, args); - if (o !== undefined) return o; - if (name === 'get_db_name') return 'f_hub1'; - return null; - }, - // Routed by shape, because three different reads go through await_query: - // the live membership, the invitee's home-root row, and the hub's owner. - async await_query(sql, ...args) { - calls.push(['query', sql, ...args]); - if (/FROM `[^`]+`\.permission/.test(sql)) { - return answers.live === undefined ? [] : [{ permission: answers.live }]; - } - if (/FROM `[^`]+`\.media/.test(sql)) { - return answers.visible ? [{ id: args[0] }] : []; - } - return [{ owner_id: 'u_owner', name: 'Marketing' }]; - }, - }; - return { - yp, - calls, - warn: () => {}, - debug: () => {}, - payload: (data, opt) => ({ ...data, ...opt }), - }; -} - -const named = (svc, re) => svc.calls.filter((c) => re.test(c[0])); - -test('an address that already has an account is granted, in HOURS', async () => { - // The stored value is an ABSOLUTE timestamp (yp_add_pending_invitation - // converts hours with TIMESTAMPADD before storing) while add_member takes - // hours from now. Handing the stored number straight over — which both old - // copies did — asks for a grant expiring in ~1.8 billion hours. - const expires = NOW() + 48 * HOUR; - const svc = fakeSvc([{ hub_id: 'h1', email: 'a@b.c', permission: 6, expiry_time: expires }]); - - const res = await resolvePendingInvitations(svc, 'a@b.c', { uid: 'u_invitee' }); - - assert.equal(res.redeemed, 1); - const add = named(svc, /\.add_member$/)[0]; - assert.deepEqual(add.slice(0, 3), ['f_hub1.add_member', 'u_invitee', 6]); - assert.ok(add[3] >= 47 && add[3] <= 48, `expiry converted back to hours, got ${add[3]}`); - assert.equal(named(svc, /^pending_invitation_delete_by_email$/).length, 1, - 'a redeemed invitation is cleared'); -}); - -test('no expiry stays permanent', async () => { - const svc = fakeSvc([{ hub_id: 'h1', email: 'a@b.c', permission: 15, expiry_time: 0 }]); - await resolvePendingInvitations(svc, 'a@b.c', { uid: 'u_invitee' }); - assert.equal(named(svc, /\.add_member$/)[0][3], 0); -}); - -test('the membership grant is followed by the hub-root permission', async () => { - const svc = fakeSvc([{ hub_id: 'h1', email: 'a@b.c', permission: 6, expiry_time: 0 }]); - await resolvePendingInvitations(svc, 'a@b.c', { uid: 'u_invitee' }); - const grants = named(svc, /\.permission_grant$/); - assert.equal(grants[0][1], '*', 'membership is the star grant'); - assert.equal(grants[0][4], 6); - assert.ok(grants.some((g) => g[1] === 'chat1'), - 'chat upload too — neither old copy made this grant, so a member who ' - + 'arrived this way could not attach a file in the workspace chat'); -}); - -test('the invitee is told over the socket, on the name the desk listens for', async () => { - sent.length = 0; - const svc = fakeSvc([{ hub_id: 'h1', email: 'a@b.c', permission: 6, expiry_time: 0 }]); - await resolvePendingInvitations(svc, 'a@b.c', { uid: 'u_invitee' }); - assert.ok(sent.some((s) => s.payload.service === 'hub.invite_received'), - 'the switcher and the sidebar both key off this exact service name'); -}); - -test('notify:false keeps an offline repair silent', async () => { - sent.length = 0; - const svc = fakeSvc([{ hub_id: 'h1', email: 'a@b.c', permission: 6, expiry_time: 0 }]); - await resolvePendingInvitations(svc, 'a@b.c', { uid: 'u_invitee', notify: false }); - assert.equal(sent.length, 0); -}); - -test('an address with NO account is left alone for account creation', async () => { - const svc = fakeSvc( - [{ hub_id: 'h1', email: 'nobody@b.c', permission: 6, expiry_time: 0 }], - { drumate_exists: null } - ); - const res = await resolvePendingInvitations(svc, 'nobody@b.c'); - assert.equal(res.redeemed, 0); - assert.equal(named(svc, /\.add_member$/).length, 0); - assert.equal(named(svc, /^pending_invitation_delete_by_email$/).length, 0, - 'the invitation is still waiting — deleting it would lose it'); -}); - -test('an invitation that expired before redemption is dropped, not granted', async () => { - const svc = fakeSvc([ - { hub_id: 'h1', email: 'a@b.c', permission: 6, expiry_time: NOW() - HOUR }, - ]); - const res = await resolvePendingInvitations(svc, 'a@b.c', { uid: 'u_invitee' }); - assert.equal(res.redeemed, 0); - assert.equal(named(svc, /\.add_member$/).length, 0); - assert.equal(named(svc, /^pending_invitation_delete_by_email$/).length, 1, - 'nothing to grant and nothing to retry'); -}); - -test('an invitation into a workspace that no longer exists is dropped', async () => { - const svc = fakeSvc( - [{ hub_id: 'gone', email: 'a@b.c', permission: 6, expiry_time: 0 }], - { get_db_name: null } - ); - const res = await resolvePendingInvitations(svc, 'a@b.c', { uid: 'u_invitee' }); - assert.equal(res.redeemed, 0); - assert.equal(named(svc, /^pending_invitation_delete_by_email$/).length, 1, - 'a dead hub must not be retried on every login'); -}); - -test('a failure keeps the row so the next attempt retries it', async () => { - const svc = fakeSvc( - [{ hub_id: 'h1', email: 'a@b.c', permission: 6, expiry_time: 0 }], - { 'f_hub1.add_member': () => { throw new Error('hub db down'); } } - ); - const res = await resolvePendingInvitations(svc, 'a@b.c', { uid: 'u_invitee' }); - assert.equal(res.redeemed, 0); - assert.equal(res.failed, 1); - assert.equal(named(svc, /^pending_invitation_delete_by_email$/).length, 0, - 'a transient failure must not silently destroy the invitation'); -}); - -test('add_member writing nothing is a failure, not a silent success', async () => { - // It answers with a db_name-carrying row only when the membership was - // actually written; anything else means it bailed, and a permission_grant on - // top of that is a grant with no membership behind it. - const svc = fakeSvc( - [{ hub_id: 'h1', email: 'a@b.c', permission: 6, expiry_time: 0 }], - { 'f_hub1.add_member': { db_name: null } } - ); - const res = await resolvePendingInvitations(svc, 'a@b.c', { uid: 'u_invitee' }); - assert.equal(res.failed, 1); - assert.equal(named(svc, /\.permission_grant$/).length, 0); -}); - -test('an empty table costs one lookup and nothing else', async () => { - const svc = fakeSvc([]); - const res = await resolvePendingInvitations(svc, 'a@b.c', { uid: 'u_invitee' }); - assert.equal(res.redeemed, 0); - assert.equal(svc.calls.length, 1, - 'this runs on every login — the common path must stop at the lookup'); -}); - -// ── the guard against walking over a membership that already works ────────── -// -// A pending row is deleted only when it is redeemed, so a stale one outlives a -// membership later granted properly through hub.invite — and the redeemer runs -// on EVERY login. Without these it would re-grant from the stale row on each -// one: demoting a privilege that was raised since, and re-running join_hub, -// whose REPLACE re-derives the home-root filename and resets its rank. - -test('a stale invitation beside a working membership writes nothing', async () => { - const svc = fakeSvc( - [{ hub_id: 'h1', email: 'a@b.c', permission: 4, expiry_time: 0 }], - { live: 15, visible: true } - ); - const res = await resolvePendingInvitations(svc, 'a@b.c', { uid: 'u_invitee' }); - assert.equal(res.alreadyMember, 1); - assert.equal(res.redeemed, 0); - assert.equal(named(svc, /\.add_member$/).length, 0, - 'an admin at 15 must not be pushed back down to the invitation\'s 4'); - assert.equal(named(svc, /\.permission_grant$/).length, 0); - assert.equal(named(svc, /^pending_invitation_delete_by_email$/).length, 1, - 'the stale row is still cleared — that is the repair'); -}); - -test('permission without the home-root row is still repaired', async () => { - // add_member writes into two databases. Half-applied is precisely the state - // this module exists to finish: the grant is there, the workspace is not. - const svc = fakeSvc( - [{ hub_id: 'h1', email: 'a@b.c', permission: 4, expiry_time: 0 }], - { live: 4, visible: false } - ); - const res = await resolvePendingInvitations(svc, 'a@b.c', { uid: 'u_invitee' }); - assert.equal(res.redeemed, 1); - assert.equal(named(svc, /\.add_member$/).length, 1, 'join_hub has to be re-run'); -}); - -test('a repair never lowers a privilege raised since the invitation', async () => { - const svc = fakeSvc( - [{ hub_id: 'h1', email: 'a@b.c', permission: 4, expiry_time: 0 }], - { live: 15, visible: false } - ); - await resolvePendingInvitations(svc, 'a@b.c', { uid: 'u_invitee' }); - assert.equal(named(svc, /\.add_member$/)[0][2], 15); - assert.equal(named(svc, /\.permission_grant$/)[0][4], 15); -}); From 0ccb857116c1432a6a3f87eafee15b19f5eae10c Mon Sep 17 00:00:00 2001 From: EddyOne81 Date: Mon, 14 Sep 2026 04:56:33 -0700 Subject: [PATCH 06/15] fix(hub): invite mail and audit lines name the workspace, not its hex id hub.add_contributors and hub.invite_with_roles mailed " added you to team 218881d8218881dc" -- and add_contributors stored that same hex id as the workspace name on the invitee's notification, where the bell feed's resolver rightly refuses to render it, so the invitation carried no workspace name at all. Real rows on stage: yp.contact_activity 727-729. A hub record has two name columns and only one of them is a name: yp.hub.name the display name a member set and sees. mfs_home returns it as `name`. yp.hub.hubname the hex id. yp.get_hub selects `IF(_exists, h.hubname, _org_name) AS name` and never exposes h.name, so BOTH a session's hub entity (which is loaded from get_hub) and a get_hub row answer the hex id to `name` AND to `hubname`. Verified on stage hub df1384eadf1384f0: yp.hub.name is "Internal Workspace", while get_hub's name and hubname are both the id, and mfs_home's name is "Internal Workspace". hub.invite already knew this and read mfs_home. The other two read the hub record. Both now resolve the name the same way, through a shared service/lib/hub-display-name.js rather than a third and fourth copy of the chain -- the drift between two copies of a workspace-name rule is exactly what produced the blank-name bug this repo fixed in August. add_contributors mfs_home was already being fetched a few lines below for the chat-upload grant; the name now reads from it. No extra query. invite_with_roles same, with the mfs_home fetch moved above the name. The get_hub call stays: it is what creates a hub's yp.disk_usage row on demand (76 of 1021 stage hubs still have none), and it supplies the last-resort fallbacks for a workspace whose yp.hub.name is NULL. Fallbacks are unchanged in spirit: a workspace with no display name still degrades to an id rather than to empty quotes in an audit line. This is the source-side half of 396e6f0. That commit made the notification render correctly whatever was stored, by resolving the LIVE name; this one stops the wrong name being written in the first place -- which also fixes the email body and the audit lines, and gives a DELETED workspace a real name to fall back on. offline/test/hub-display-name.test.js -- 26 cases and 4 negative controls. Each of the three call sites has its own `hubname` expression sliced out of hub.js and executed against stubs, so the shipped expressions are what runs, plus an invariant that no invite path may grow its own copy of the chain again. All 38 suites green; the workflow's ESLint ruleset clean. Co-Authored-By: Claude Opus 5 (1M context) --- offline/test/hub-display-name.test.js | 235 ++++++++++++++++++++++++++ service/lib/hub-display-name.js | 50 ++++++ service/private/hub.js | 46 +++-- 3 files changed, 321 insertions(+), 10 deletions(-) create mode 100644 offline/test/hub-display-name.test.js create mode 100644 service/lib/hub-display-name.js diff --git a/offline/test/hub-display-name.test.js b/offline/test/hub-display-name.test.js new file mode 100644 index 00000000..6835aca6 --- /dev/null +++ b/offline/test/hub-display-name.test.js @@ -0,0 +1,235 @@ +#!/usr/bin/env node +// +// hub-display-name.test.js +// +// A workspace has two names on its hub record and only one of them is a name: +// +// yp.hub.name the display name a member set and sees. mfs_home returns it. +// yp.hub.hubname the hex id. yp.get_hub selects +// `IF(_exists, h.hubname, _org_name) AS name`, so a session's +// hub entity AND a get_hub row BOTH answer the hex id to +// `name` and to `hubname`. +// +// hub.invite read mfs_home. hub.add_contributors and hub.invite_with_roles read +// the hub record, so they mailed " added you to team 218881d8218881dc" +// and add_contributors stored that hex id as the workspace name on the invitee's +// notification -- where the bell feed's resolver rightly refuses to render it, +// leaving the invitation with no workspace name at all (real stage rows 727-729). +// +// node offline/test/hub-display-name.test.js +// +// The resolver is required for real. Each of the three call sites has its own +// `hubname` expression SLICED OUT of service/private/hub.js and evaluated against +// stubs, so this tests the shipped call sites rather than a retyped copy of them. +// +// Exit code 0 = all pass, 1 = any failure. + +const fs = require('fs'); +const path = require('path'); +const crypto = require('crypto'); + +const HUB_FILE = path.join(__dirname, '..', '..', 'service', 'private', 'hub.js'); +const src = fs.readFileSync(HUB_FILE, 'utf8'); +const MD5_BEFORE = crypto.createHash('md5').update(src).digest('hex'); + +const { resolveHubDisplayName } = require('../../service/lib/hub-display-name'); + +let passed = 0; +let failed = 0; +const failures = []; +function check(label, got, want) { + if (JSON.stringify(got) === JSON.stringify(want)) { passed++; return true; } + failed++; + failures.push(`${label}\n got ${JSON.stringify(got)}\n want ${JSON.stringify(want)}`); + return false; +} + +const HEX = '218881d8218881dc'; + +// --------------------------------------------------------------------------- +// 1. The resolver itself. +// --------------------------------------------------------------------------- +{ + check('A1 the display name wins', + resolveHubDisplayName({ name: 'Marketing' }, HEX, HEX, HEX), 'Marketing'); + check('A2 no mfs_home row falls through to the fallbacks', + resolveHubDisplayName(null, '', HEX), HEX); + check('A3 an mfs_home row without a name falls through too', + resolveHubDisplayName({ chat_upload_id: 'x' }, HEX), HEX); + check('A4 a NULL display name falls through', + resolveHubDisplayName({ name: null }, HEX), HEX); + check('A5 an empty display name is not a name', + resolveHubDisplayName({ name: '' }, HEX), HEX); + check('A6 fallbacks are tried in order', resolveHubDisplayName(null, null, '', 'b', 'c'), 'b'); + check('A7 nothing at all yields null, never undefined', + resolveHubDisplayName(null), null); + check('A8 nothing at all with empty fallbacks yields null', + resolveHubDisplayName(undefined, null, '', 0), null); + // The whole point: a hex id must never outrank a real name. + check('A9 the hex id never beats the display name', + resolveHubDisplayName({ name: 'Marketing' }, HEX), 'Marketing'); +} + +// --------------------------------------------------------------------------- +// 2. The three call sites, sliced out of hub.js and executed. +// +// Each site's `const hubname = resolveHubDisplayName( ... );` is extracted by +// paren-matching and evaluated with stubs, so what runs here is the expression +// that ships. +// --------------------------------------------------------------------------- +function sliceHubnameExpr(methodMarker, what) { + const mStart = src.indexOf(methodMarker); + if (mStart < 0) { + console.error(`FATAL: ${what} not found in ${HUB_FILE} — fix this slicer`); + process.exit(1); + } + const at = src.indexOf('const hubname = resolveHubDisplayName(', mStart); + if (at < 0) { + console.error(`FATAL: ${what} does not resolve its name through the shared` + + ' resolver — that is the bug this file exists to prevent'); + process.exit(1); + } + let depth = 0; + let end = -1; + for (let i = src.indexOf('(', at); 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 parens slicing ${what}`); process.exit(1); } + return { expr: src.slice(at, end) + ';', at, mStart }; +} + +// `mfs_home` MUST be read before the name, or the const is in its temporal dead +// zone and the invitation throws. Checked per site, not assumed. +function mfsHomeComesFirst(methodMarker, nameAt) { + const mStart = src.indexOf(methodMarker); + const fetch = src.indexOf('mfs_home = await', mStart); + return fetch > 0 && fetch < nameAt; +} + +const SITES = [ + { + name: 'hub.invite', + marker: '\n async invite() {', + // this.hub is an Entity; get(Attr.hubname) and get(Attr.name) both answer + // the hex id in production, which is exactly what is stubbed here. + run(expr, mfs_home) { + const ctx = { hub: { get: () => HEX } }; + const Attr = { hubname: 'hubname', name: 'name', id: 'id' }; + return (new Function('Attr', 'resolveHubDisplayName', 'mfs_home', 'hubId', 'ctx', + `${expr.replace(/this\.hub/g, 'ctx.hub')} + return hubname;`))(Attr, resolveHubDisplayName, mfs_home, HEX, ctx); + }, + }, + { + name: 'hub.add_contributors', + marker: '\n async add_contributors() {', + run(expr, mfs_home) { + const ctx = { hub: { get: () => HEX } }; + const Attr = { hubname: 'hubname', name: 'name', id: 'id' }; + return (new Function('Attr', 'resolveHubDisplayName', 'mfs_home', 'ctx', + `${expr.replace(/this\.hub/g, 'ctx.hub')} + return hubname;`))(Attr, resolveHubDisplayName, mfs_home, ctx); + }, + }, + { + name: 'hub.invite_with_roles', + marker: '\n async invite_with_roles() {', + run(expr, mfs_home) { + // get_hub answers the hex id on BOTH name columns. + const hubInfo = { hubname: HEX, name: HEX }; + return (new Function('resolveHubDisplayName', 'mfs_home', 'hubInfo', 'hub_id', + `${expr} + return hubname;`))(resolveHubDisplayName, mfs_home, hubInfo, HEX); + }, + }, +]; + +for (const site of SITES) { + const { expr, at } = sliceHubnameExpr(site.marker, site.name); + check(`B ${site.name} names a real workspace by its display name`, + site.run(expr, { name: 'Marketing' }), 'Marketing'); + check(`C ${site.name} never mails the hex id when a display name exists`, + site.run(expr, { name: 'Marketing' }) === HEX, false); + check(`D ${site.name} still degrades to an id rather than to nothing`, + site.run(expr, null), HEX); + check(`E ${site.name} reads mfs_home BEFORE resolving the name`, + mfsHomeComesFirst(site.marker, at), true); +} + +// --------------------------------------------------------------------------- +// 3. Anti-drift: no invite path may grow its own copy of the chain again. +// --------------------------------------------------------------------------- +{ + const hubnameAssignments = src.match(/const hubname = [^\n]*/g) || []; + check('F1 every `const hubname =` in hub.js goes through the resolver, first', + hubnameAssignments.every( + (l) => l.trim().startsWith('const hubname = resolveHubDisplayName(')), true); + check('F2 and there are exactly the three invite sites', hubnameAssignments.length, 3); + check('F3 hub.js requires the shared resolver', + /require\(["']\.\.\/lib\/hub-display-name["']\)/.test(src), true); + // The resolver is the only place allowed to know the fallback order. + const lib = fs.readFileSync( + path.join(__dirname, '..', '..', 'service', 'lib', 'hub-display-name.js'), 'utf8'); + check('F4 the resolver reads the display name first', + /const live = mfsHome && mfsHome\.name;/.test(lib), true); +} + +// --------------------------------------------------------------------------- +// 4. Negative controls: put each mistake back and prove a named case goes red. +// --------------------------------------------------------------------------- +{ + const controls = [ + ['NC1 the hub record outranks the display name (the original bug)', + (mfsHome, ...fb) => { + for (const v of fb) if (v) return v; + return (mfsHome && mfsHome.name) || null; + }, + (f) => f({ name: 'Marketing' }, HEX) === 'Marketing'], + ['NC2 an empty display name counts as a name', + (mfsHome, ...fb) => { + if (mfsHome && mfsHome.name !== undefined) return mfsHome.name; + for (const v of fb) if (v) return v; + return null; + }, + (f) => f({ name: '' }, HEX) === HEX], + ['NC3 no fallback at all (an audit line with empty quotes)', + (mfsHome) => (mfsHome && mfsHome.name) || null, + (f) => f(null, HEX) === HEX], + ]; + let caught = 0; + for (const [label, broken, stillHolds] of controls) { + if (stillHolds(broken)) { + console.log(` ✗ ${label} — NOT CAUGHT`); + failed++; failures.push(label + ' — NOT CAUGHT'); + } else { + caught++; console.log(` ✓ ${label} — CAUGHT`); + } + } + // And one that mutates the SOURCE: a site that grows its own chain again must + // break F1, the invariant that is the whole reason the resolver was extracted. + { + const mutated = src.replace( + 'const hubname = resolveHubDisplayName(', + 'const hubname = (mfs_home && mfs_home.name) || (', + ); + const lines = mutated.match(/const hubname = [^\n]*/g) || []; + const stillClean = lines.length === 3 + && lines.every((l) => l.trim().startsWith('const hubname = resolveHubDisplayName(')); + if (mutated !== src && !stillClean) { + caught++; console.log(' ✓ NC4 a site grows its own copy of the chain — CAUGHT'); + } else { + console.log(' ✗ NC4 a site grows its own copy of the chain — NOT CAUGHT'); + failed++; failures.push('NC4 — NOT CAUGHT'); + } + } + console.log(''); + check('Z1 the working tree was never modified', + crypto.createHash('md5').update(fs.readFileSync(HUB_FILE, 'utf8')).digest('hex'), + MD5_BEFORE); + console.log(`negative controls caught: ${caught}`); +} + +for (const f of failures) console.log(' ✗ ' + f); +console.log(`\n${passed} passed, ${failed} failed.`); +process.exit(failed ? 1 : 0); diff --git a/service/lib/hub-display-name.js b/service/lib/hub-display-name.js new file mode 100644 index 00000000..69d705ce --- /dev/null +++ b/service/lib/hub-display-name.js @@ -0,0 +1,50 @@ +/** + * @license + * Copyright 2024 Thidima SA. All Rights Reserved. + * Licensed under the GNU AFFERO GENERAL PUBLIC LICENSE, Version 3 + */ + +/** + * The name a workspace should be CALLED BY when we write about it -- an invite + * email, the message that rides the invitation, an audit line, the workspace + * name stored on a notification. + * + * There is exactly one source for it: **`yp.hub.name`**, which `mfs_home` + * returns as `name`. Every other candidate on a hub record is the hex id + * wearing a friendly column name, and that is the whole reason this exists: + * + * yp.hub.hubname the hex id. Entity.get(Attr.name) resolves to it, + * because a session's `hub` is loaded from yp.get_hub + * and that procedure selects + * `IF(_exists, h.hubname, _org_name) AS name` -- + * it never exposes h.name at all. + * get_hub().hubname/.name same two columns, same trap, one call further away. + * + * So `hub.invite` read mfs_home and mailed "Lexis added you to team Marketing", + * while `hub.add_contributors` and `hub.invite_with_roles` read the hub record + * and mailed "Lexis added you to team 218881d8218881dc" -- and + * add_contributors stored that same hex id as the workspace name on the + * invitee's notification, where it is worse than useless: the bell feed's + * resolver deliberately refuses to render a hex id as a label, so the invite + * showed no workspace name at all. + * + * The fallbacks are kept because they are what the working call site already + * did: a hub whose `yp.hub.name` is genuinely NULL has no display name, and an + * id is still better than an empty quote in an audit line. They are a last + * resort, never the answer when a real name exists. + * + * @param {object} mfsHome the target hub's `mfs_home` row (its `name` is + * yp.hub.name). Anything falsy is simply skipped. + * @param {...*} fallbacks tried in order when there is no display name. + * @returns {*} the name to write, or null when even the fallbacks are empty. + */ +function resolveHubDisplayName(mfsHome, ...fallbacks) { + const live = mfsHome && mfsHome.name; + if (live) return live; + for (const value of fallbacks) { + if (value) return value; + } + return null; +} + +module.exports = { resolveHubDisplayName }; diff --git a/service/private/hub.js b/service/private/hub.js index c19a8801..5f1af3a3 100644 --- a/service/private/hub.js +++ b/service/private/hub.js @@ -32,6 +32,7 @@ const { notifyMemberJoined, notifyMembersChanged } = require("../lib/notify-memb const { butlerFrom } = require("../lib/mail-sender"); const { mailFailure } = require("../lib/mail-result"); const { resolveHubInviteName } = require("../lib/hub-invite-name"); +const { resolveHubDisplayName } = require("../lib/hub-display-name"); const { MfsTools } = require("@drumee/server-core"); const { remove_dir } = MfsTools; const { toArray } = utils; @@ -719,13 +720,24 @@ class __private_hub extends Hub { async add_contributors() { let users = this.input.need(Attr.users); const username = this.user.get("fullname"); - const hubname = this.hub.get(Attr.name); const privilege = this.input.use(Attr.privilege) || this.hub.get(Attr.settings).default_privilege; const hours = this.input.use(Attr.hours, 0) const days = this.input.use(Attr.days, 0); const expiry = hours * 1 + days * 24; const lang = this.user.language() || this.input.app_language(); let mfs_home = await this.db.await_proc("mfs_home"); + // This used to be this.hub.get(Attr.name) alone, which is yp.hub.hubname -- + // the HEX ID. Everything downstream inherited it: the invitation email, the + // two audit lines, and (through _grantMembership) the workspace name stored + // on the invitee's notification, where a hex id renders as no name at all. + // Same resolver and same chain as invite(); mfs_home was already fetched + // here for the chat-upload grant, so this costs no extra query. + const hubname = resolveHubDisplayName( + mfs_home, + this.hub.get(Attr.hubname), + this.hub.get(Attr.name), + this.hub.get(Attr.id), + ); let msg = Cache.message("_x_add_you_to_team", lang).format( username, hubname @@ -1369,11 +1381,15 @@ class __private_hub extends Hub { const hubId = this.hub.get(Attr.id); // mfs_home reads yp.hub.name directly (the actual display name); // this.hub.get(Attr.name/hubname) returns yp.hub.hubname (a technical id). + // The chain itself now lives in service/lib/hub-display-name.js, because + // the other two invite endpoints got it wrong by each carrying their own. const mfs_home = await this.db.await_proc("mfs_home"); - const hubname = (mfs_home && mfs_home.name) - || this.hub.get(Attr.hubname) - || this.hub.get(Attr.name) - || hubId; + const hubname = resolveHubDisplayName( + mfs_home, + this.hub.get(Attr.hubname), + this.hub.get(Attr.name), + hubId, + ); const area = this.hub.get(Attr.area); // The ONE axis the email body varies on: internal (private) vs external // (shared) workspace. Also decides whether the workspace preview is redacted. @@ -1953,14 +1969,24 @@ class __private_hub extends Hub { continue; } - // Hub display name for notification message + // mfs_home is needed for the chat_upload_id permission grant below, and + // it is also the ONLY thing here that knows the workspace's display name: + // get_hub returns `IF(_exists, h.hubname, _org_name) AS name`, so both of + // its name columns are the hex id. Reading them is why this endpoint used + // to mail " added you to team 218881d8218881dc". Fetched before + // the name now; the get_hub call stays because it is what creates a hub's + // yp.disk_usage row on demand, and it still supplies the last-resort + // fallbacks for a workspace whose yp.hub.name is genuinely NULL. + const mfs_home = await this.yp.await_proc(`${hub_db}.mfs_home`); const hubInfo = await this.yp.await_proc('get_hub', hub_id); - const hubname = (hubInfo && (hubInfo.hubname || hubInfo.name)) || hub_id; + const hubname = resolveHubDisplayName( + mfs_home, + hubInfo && hubInfo.hubname, + hubInfo && hubInfo.name, + hub_id, + ); const msg = Cache.message('_x_add_you_to_team', lang).format(username, hubname); - // mfs_home needed for chat_upload_id permission grant - const mfs_home = await this.yp.await_proc(`${hub_db}.mfs_home`); - const members = []; // UIDs to add immediately const rows = []; // Results from add_member (for WebSocket notify) // uid -> address it was invited at, for invite_track. The immediate-grant From 6a622a2f26cb81acb19d3df0ea5f0c87a49b6164 Mon Sep 17 00:00:00 2001 From: EddyOne81 Date: Mon, 14 Sep 2026 05:07:15 -0700 Subject: [PATCH 07/15] =?UTF-8?q?docs(activity):=20record=20the=20second?= =?UTF-8?q?=20half=20of=20the=20invite=20report=20=E2=80=94=20the=20dead?= =?UTF-8?q?=20click?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Duy passed on the rest of Lexis's report: the people who got "Lexis wants to connect" also said clicking the notification led nowhere. Same row, same cause — the row's COPY and its click ROUTER both switch on the resolved category — so 396e6f0 already fixes it. Verified, not assumed: before category 'contact' -> #/desk/wm/contact&ts=… -> wm case 'contact' -> Desk.openContactPanel(), which opens Contacts on [Pending]. A workspace invite is not a contact request, so that tab has nothing in it: the click appears to do nothing. after category 'hub_invite' -> #/desk/wm/reveal/?hub_id=…&nid=0&filetype=folder&pid=0&ts=… -> wm case 'reveal' -> openNotificationLocation(), which docks the workspace. And a SECOND, independent reason it was dead, which is why the hub_id stamp is load-bearing rather than cosmetic: openNotificationLocation opens with `if (!hub_id) { warn; return; }`, and activity_get_feed_all sets hub_id NULL on every contact row. Without the stamp the click would have bailed there even with the category fixed. Both hashes are produced by executing the two click branches sliced out of ui-team's panel/activity/widget/item/index.js, parsed with the real parseModule from @drumee/ui-core, and matched against the real wm route table — see ~/invite-noti-category-render-test.mjs, now 20 cases. This commit is comments only; no behaviour change. Co-Authored-By: Claude Opus 5 (1M context) --- offline/test/hub-invite-feed-row.test.js | 6 +++++- service/private/activity.js | 12 ++++++++---- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/offline/test/hub-invite-feed-row.test.js b/offline/test/hub-invite-feed-row.test.js index 2a52665a..738f02ee 100644 --- a/offline/test/hub-invite-feed-row.test.js +++ b/offline/test/hub-invite-feed-row.test.js @@ -8,7 +8,9 @@ // is the panel's DEFAULT -- returns the invite as a bare yp.contact_activity row: // `category` NULL, `event_type` 'contact', `event` 'hub_invite_received'. The // client resolves a row's category as `category || event_type || type`, so the -// row took the contact-request branch of the renderer. +// row took the contact-request branch of the renderer -- and its click ROUTER +// switches on the same value, which is why the recipients also reported that +// clicking the notification led nowhere. // // node offline/test/hub-invite-feed-row.test.js // @@ -186,6 +188,8 @@ async function suite(stampSrc = STAMP_SRC, quiet = false) { C('A1 category is stamped', r.category, 'hub_invite'); C('A2 the client no longer resolves it as a contact request', clientCategory(r), 'hub_invite'); + // Load-bearing for the "clicking it led nowhere" half of the report: the + // desk's notification opener refuses a row that carries no hub_id. C('A3 workspace id is surfaced for the click target', r.hub_id, HUB); C('A4 the inviter owns the avatar', r.author_id, INVITER); C('A5 the sentence gets the workspace name', r.hub_name, 'Marketing'); diff --git a/service/private/activity.js b/service/private/activity.js index e33eba2f..e34d1077 100644 --- a/service/private/activity.js +++ b/service/private/activity.js @@ -1458,7 +1458,9 @@ class MfsActivity extends Entity { * Make a workspace invitation READ as one in the chronological feed. * * THE BUG (Lexis, 2026-09-14): "[Username] invited you to [workspace]" was - * rendered as "[Username] wants to connect" -- the CONTACT-request copy. + * rendered as "[Username] wants to connect" -- the CONTACT-request copy -- + * and the recipients reported that clicking it led nowhere. One cause: the + * row's copy AND its click router both switch on the same resolved category. * * WHY. A workspace invite is a yp.contact_activity row (event * 'hub_invite_received', written by hub._grantMembership). Under Unread OFF -- @@ -1488,9 +1490,11 @@ class MfsActivity extends Entity { * contact_activity id. Now they dismiss as contact events, which * is the table the row actually lives in. * hub_id the invited workspace, from the row's own `data` JSON. - * activity_get_feed_all sets hub_id NULL on every contact row, - * and the invite row is the one place the client needs it (the - * click opens that workspace). Same trick flattenTaskFields uses. + * activity_get_feed_all sets hub_id NULL on every contact row. + * This is the SECOND, independent reason the click was dead: the + * desk's notification opener (wm openNotificationLocation) starts + * with `if (!hub_id) return`, so even with the category fixed the + * click would have bailed there. Same trick flattenTaskFields uses. * author_id the inviter, so the card shows THEIR face. Without it * getAuthorId() falls through to undefined and the avatar * defaults to the viewer's own picture. From d0f14974617ee4ab969328a0b0775dce33af1ee7 Mon Sep 17 00:00:00 2001 From: EddyOne81 Date: Mon, 14 Sep 2026 21:17:29 -0700 Subject: [PATCH 08/15] fix(activity): a copy row names the folder it was copied FROM MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The folder chip never appeared on a media.copy notification, and the reason is a precedence rule that is correct for every other media event and backwards for this one. _stampFolderNames reads the parent id from `dest` first, then `src` — right for an upload or a move, where dest is where the file ended up in the workspace being notified. A copy is filed against the SOURCE hub, so `hubId` resolves to the workspace the file was copied OUT of, while `dest.parent_id` is a folder in the copier's own personal space. The lookup therefore paired a destination node id with the source hub id, matched nothing, and the chip was silently dropped. Had those ids ever collided it would have been worse than a missing chip: the row would have named a folder the reader has no access to. media.copy now reads src first and still falls back to dest, because a handful of prod rows carry an empty `src` object. Every other event keeps dest precedence untouched. Covered by three new cases in activity-folder-name.test.js, which slices the real method out of this file rather than copying it. Verified they fail without the change (the chip comes back named "WRONG - copier private folder") and pass with it: 44 passed, 0 failed. Co-Authored-By: Claude Opus 5 (1M context) --- offline/test/activity-folder-name.test.js | 50 +++++++++++++++++++++++ service/private/activity.js | 13 +++++- 2 files changed, 61 insertions(+), 2 deletions(-) diff --git a/offline/test/activity-folder-name.test.js b/offline/test/activity-folder-name.test.js index 6a89c9ee..e85c0be5 100644 --- a/offline/test/activity-folder-name.test.js +++ b/offline/test/activity-folder-name.test.js @@ -138,6 +138,56 @@ const upload = (over = {}) => ({ eq(rows[0].folder_name, 'destWins', 'dest keeps precedence when it has data'); } + { + // REGRESSION (Lexis, 2026-09-14): a media.copy row must read its parent + // from `src`, the OPPOSITE of every other media event. + // + // A copy is filed against the SOURCE hub, so `hub_id` here is the workspace + // the file was copied OUT of -- while `dest.parent_id` is a folder in the + // copier's own space. Preferring dest paired 'HUB1' with 'PDEST', a pair + // that exists nowhere, so the lookup matched nothing and the row lost its + // chip. Worse, had the ids ever collided it would have named a folder the + // reader cannot see. + const h = new Holder({ + 'HUB1:PSRC': { filename: 'No more lies' }, + 'HUB1:PDEST': { filename: 'WRONG - copier private folder' }, + }); + const rows = [{ + event: 'media.copy', + hub_id: 'HUB1', + src: JSON.stringify({ filename: 'MCCP_B1.pdf', parent_id: 'PSRC', hub_id: 'HUB1' }), + dest: JSON.stringify({ filename: 'MCCP_B1.pdf', parent_id: 'PDEST', hub_id: 'PERSONAL' }), + }]; + await h._stampFolderNames(rows); + eq(rows[0].folder_name, 'No more lies', 'copy names the folder it was copied FROM'); + eq(h.calls[0].id, 'PSRC', 'looked up the SOURCE parent, never dest'); + } + { + // A copy with no usable src still falls back to dest rather than giving up: + // a handful of prod rows carry an empty `src` object. + const h = new Holder({ 'HUB1:PDEST': { filename: 'fallback' } }); + const rows = [{ + event: 'media.copy', + hub_id: 'HUB1', + src: '{}', + dest: JSON.stringify({ parent_id: 'PDEST' }), + }]; + await h._stampFolderNames(rows); + eq(rows[0].folder_name, 'fallback', 'empty src falls back to dest'); + } + { + // And the flip side: a NON-copy media event must keep dest precedence, + // which is what a move depends on. + const h = new Holder({ 'HUB1:PD': { filename: 'destWins' } }); + const rows = [upload({ + event: 'media.relocate', + dest: JSON.stringify({ parent_id: 'PD' }), + src: JSON.stringify({ parent_id: 'PS' }), + })]; + await h._stampFolderNames(rows); + eq(rows[0].folder_name, 'destWins', 'non-copy events are unchanged'); + } + console.log('\n3. lookups are deduped, not per row'); { const h = new Holder({ 'HUB1:P1': { filename: 'checkin' } }); diff --git a/service/private/activity.js b/service/private/activity.js index e34d1077..f82970bd 100644 --- a/service/private/activity.js +++ b/service/private/activity.js @@ -1359,8 +1359,17 @@ class MfsActivity extends Entity { // silently left every changelog row without a chip. const dest = asObject(r.dest) || {}; const source = asObject(r.src) || {}; - const parentId = r.parent_id || dest.parent_id || dest.pid - || source.parent_id || source.pid; + // media.copy has to read the other way round. The row is filed against + // the SOURCE hub, so `hubId` below resolves to the source workspace -- + // but `dest.parent_id` is a folder in the COPIER's own space. Pairing the + // two looked up a destination node id inside the source hub, matched + // nothing, and left every copy row without a chip. The folder these + // readers care about is the one the file was copied FROM. + const fromSource = r.event === 'media.copy'; + const parentId = r.parent_id + || (fromSource + ? (source.parent_id || source.pid || dest.parent_id || dest.pid) + : (dest.parent_id || dest.pid || source.parent_id || source.pid)); const hubId = r.hub_id || dest.hub_id || source.hub_id; if (!parentId || !hubId || `${parentId}` === '0') continue; const key = `${hubId}:${parentId}`; From a1ebd1382af154d88a94eef6195415ed2bf2cd4f Mon Sep 17 00:00:00 2001 From: Tran Hoang Huan <121786621+tranh0anghuan@users.noreply.github.com> Date: Tue, 15 Sep 2026 16:45:52 +0700 Subject: [PATCH 09/15] feat(page): the warm-up screen uses the exported drumee lockup (#220) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The boot screen drew the cloud mark as inline SVG beside a drumee set in Armin Grotesk. It now carries the exported lockup — the same asset the sign-in card shows (signin/src/assets/drumee-logo.svg, via .signin-form__logo-image) — so boot and sign-in state the brand identically, and the wordmark stops depending on a webfont that may not have arrived yet on a cold load. INLINED rather than : this screen is on display precisely BECAUSE the network is busy fetching the app bundles, and a logo needing its own request could land after the screen it belongs to is gone. Sizing, the caption colour and the caption/bar gap are set in this file's own
- drumee
From e4de1d2d314d03938300f219869922948d12a208 Mon Sep 17 00:00:00 2001 From: EddyOne81 Date: Thu, 17 Sep 2026 01:29:02 -0700 Subject: [PATCH 10/15] fix(permission): hub.delete_hub asks for admin, not owner MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Lexis, via Duy, 2026-09-17, as the second half of the workspace-delete rule whose client side shipped yesterday (ui-team 14c76cb9, which gates the Folder Setting panel's Delete row on the admin bit). THE OWNER BAR WAS NEVER A BOUNDARY, which is what makes this safe rather than a widening. A workspace admin could already reach owner without it: hub.change_owner src: admin. Takes an arbitrary uid, demotes EVERY current owner to 31, writes 63 for the target and updates yp.hub.owner_id. One call. hub.set_privilege src: admin. Passes the caller's `privilege` input straight into permission_set(uid, priv), which UPDATEs whatever it is handed. No clamp, so 63 is writable. hub.delete_contributor src: admin. Its only guard is `uid != this.uid`, so an admin can expel the owner. So `src: owner` here cost an admin one extra request and cost every legitimate admin a 403 on a row the client offers them. It granted nobody protection. WHAT DOES NOT CHANGE, all checked rather than assumed: - The read-only clamps still catch it. router/rest/index.js keys on `permission.src > READ_LEVEL`, never on a service name; admin (16) clears that bar exactly as owner (32) did, so secure-share recipients carrying a session ceiling and over-limit domains are refused as before. delete_hub is not in OVER_LIMIT_MUTATING_ALLOWLIST and still is not. - Personal workspaces cannot come through: delete_hub refuses a non-hub entity type before anything is destroyed. - Org/domain admins gain nothing. Workspace admin is always an explicit per-hub grant; nothing auto-grants the admin bit across an org. - Billing is untouched. entity_delete never reads yp.quota, and subscriptions key on domain_id/payer_id, not on a hub. It removes the hub's disk_usage row, which is correct. - media.merge_workspace and media.copy_workspace STAY on owner. Their doc cited "the bar hub.delete_hub uses", which would now read as admin, so the cross-reference is corrected in place to say they are deliberately stricter and are not to be aligned without a decision of their own. WHAT DOES CHANGE, and is worth knowing: deleting a workspace is irreversible — entity_delete DROPs the database and the directory removal runs detached, with no trash and no backup — and the audit line is written to the DELETER's drumate because the hub DB is about to go. delete_hub also broadcasts only to entity_sockets(hub_id), so an OFFLINE owner is told nothing. Nobody gained a capability here, but the button is one click away for more people, so notifying the owner is the obvious follow-up and is NOT in this commit. offline/test/hub-delete-permission.test.js pins the two properties that made this safe — Edit/Chat/View still refused, and src still above `read` — plus the two siblings that must stay on owner. It reads the bitmasks from the installed @drumee/server-essentials rather than a copy, and it is wired into the CI regression job because it guards a permission bar. Verified RED on the pre-change ACL. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/test.yml | 6 ++ acl/hub.json | 3 +- acl/media.json | 2 +- offline/test/hub-delete-permission.test.js | 113 +++++++++++++++++++++ 4 files changed, 122 insertions(+), 2 deletions(-) create mode 100644 offline/test/hub-delete-permission.test.js diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 9cd3a21d..4bcb3411 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -41,3 +41,9 @@ jobs: run: node offline/test/media-search-names.test.js - name: Run factory projection-readiness regression test run: node --test offline/test/factory-projection-readiness.test.js + # Guards a PERMISSION bar, so it earns a CI step rather than sitting in + # offline/test unrun: it pins that Edit still cannot delete a workspace, + # and that delete_hub stays above `read` so the DMZ read-only ceiling and + # the over-limit clamp keep catching it. + - name: Run workspace-delete permission regression test + run: node --test offline/test/hub-delete-permission.test.js diff --git a/acl/hub.json b/acl/hub.json index 68de051e..d8ef1fa7 100644 --- a/acl/hub.json +++ b/acl/hub.json @@ -580,9 +580,10 @@ } }, "delete_hub": { + "doc": "Destroy a workspace: drops its database and deletes its files on disk. IRREVERSIBLE - there is no trash, no backup and no undo, and the directory removal runs detached. Asks ADMIN, not owner, since 2026-09-17 (Lexis, via Duy). The owner bar it used to carry was never a real boundary: hub.change_owner and hub.set_privilege are themselves admin-gated and neither clamps what it writes, so any workspace admin could already take ownership in a single call and then delete. Requiring owner here only added a step, while leaving the client offering a Delete row that answered 403 - the row is gated on the admin bit, see ui-team window/folder/skeleton/settings-action-panel.js. Still refuses a non-hub entity type, so a personal workspace cannot come through here.", "scope": "hub", "permission": { - "src": "owner" + "src": "admin" } }, "get_action_log": { diff --git a/acl/media.json b/acl/media.json index 480bf33a..4eb5d9ea 100644 --- a/acl/media.json +++ b/acl/media.json @@ -1834,7 +1834,7 @@ ] }, "merge_workspace": { - "doc": "Merge one workspace into another. Everything at the top level of the source workspace is moved into a new folder, named after the source, inside the destination. THE SOURCE WORKSPACE SURVIVES, EMPTIED - nothing is deleted and no database is dropped. Only files and folders cross. Chat threads stay in the database that owns them, the same rule workspace_move applies; tasks stay because task.nid addresses media rows in the source database; meetings, share links, trash and version history stay too. Members do NOT cross - the destination member list is untouched, so anyone who reached these files only through the source workspace loses them. That is the same outcome as moving a single file across workspaces today, and it keeps granting access an explicit admin-gated act rather than a side effect of a move. Asks owner on the source because it empties somebody whole workspace, the bar hub.delete_hub uses; asks only write on the destination, the bar workspace_move uses to receive files, because it adds nobody to it.", + "doc": "Merge one workspace into another. Everything at the top level of the source workspace is moved into a new folder, named after the source, inside the destination. THE SOURCE WORKSPACE SURVIVES, EMPTIED - nothing is deleted and no database is dropped. Only files and folders cross. Chat threads stay in the database that owns them, the same rule workspace_move applies; tasks stay because task.nid addresses media rows in the source database; meetings, share links, trash and version history stay too. Members do NOT cross - the destination member list is untouched, so anyone who reached these files only through the source workspace loses them. That is the same outcome as moving a single file across workspaces today, and it keeps granting access an explicit admin-gated act rather than a side effect of a move. Asks owner on the source because it empties somebody whole workspace - deliberately stricter than hub.delete_hub, which moved to admin on 2026-09-17, and not to be aligned with it without a decision of its own; asks only write on the destination, the bar workspace_move uses to receive files, because it adds nobody to it.", "scope": "hub", "permission": { "src": "owner", diff --git a/offline/test/hub-delete-permission.test.js b/offline/test/hub-delete-permission.test.js new file mode 100644 index 00000000..c974b62f --- /dev/null +++ b/offline/test/hub-delete-permission.test.js @@ -0,0 +1,113 @@ +#!/usr/bin/env node + +/** + * @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. + * ============================================================================= + */ + +/** + * `hub.delete_hub` asks for ADMIN, and the blast radius that decision rests on. + * + * Lexis, via Duy, 2026-09-17. The owner bar this service used to carry was never + * a real boundary: `hub.change_owner` and `hub.set_privilege` are themselves + * `src: admin` and neither clamps what it writes, so any workspace admin could + * already take ownership in a single call and then delete. Requiring owner only + * added a step, while leaving the client offering a Delete row that answered + * 403 (ui-team gates that row on the admin bit). + * + * What is pinned here is not the constant — it is the two properties that made + * widening it safe, either of which a later edit could quietly take away: + * + * 1. Edit / Chat / View are still refused. That is the whole point of the + * report this came from. + * 2. `delete_hub` still sits ABOVE `read`, which is what keeps the DMZ / + * secure-share read-only ceiling and the over-limit clamp catching it + * (router/rest/index.js `mightMutate`). Owner and admin both clear that + * bar; anything at or below `read` would not. + * + * The bitmask values come from the INSTALLED @drumee/server-essentials, not + * from a copy, so a change to the shared table is a failure here rather than a + * silent drift. + */ +const test = require("node:test"); +const assert = require("node:assert"); +const { readFileSync } = require("node:fs"); +const { join } = require("node:path"); + +const { permissionValue } = require("@drumee/server-essentials"); + +const ACL = (name) => + JSON.parse(readFileSync(join(__dirname, "..", "..", "acl", `${name}.json`), "utf8")) + .services; + +// The stored privilege WORDS a member can hold — hub.set_privilege writes these +// and user_permission() hands them back. Kept literal on purpose: they are the +// thing under test, so deriving them from the same table would prove nothing. +const ROLE = { view: 0b0000011, chat: 0b0000111, edit: 0b0001111, admin: 0b0011111, owner: 0b0111111 }; + +// Exactly what lib/acl.js check_source does with each row. +const granted = (privilege, asked) => !!(privilege & asked); + +test("hub.delete_hub asks for the admin bit", () => { + const spec = ACL("hub").delete_hub; + assert.equal(spec.permission.src, "admin"); + assert.equal(spec.scope, "hub"); + assert.equal(permissionValue(spec.permission.src), 0b0010000); +}); + +test("Admin and Owner may delete; Edit, Chat and View may not", () => { + const asked = permissionValue(ACL("hub").delete_hub.permission.src); + for (const [role, want] of Object.entries({ + view: false, chat: false, edit: false, admin: true, owner: true, + })) { + assert.equal( + granted(ROLE[role], asked), want, + `${role} (${ROLE[role]}) got the wrong answer against asked=${asked}`, + ); + } +}); + +test("delete_hub still sits above `read`, so the read-only clamps still catch it", () => { + // router/rest/index.js: mightMutate = permission.src > READ_LEVEL. A + // secure-share recipient carries a read-only session ceiling and an + // over-limit domain is clamped to reads; both rely on this comparison, NOT on + // a service name, so widening owner -> admin must not cross that line. + const READ = permissionValue("read"); + const src = permissionValue(ACL("hub").delete_hub.permission.src); + assert.ok(src > READ, "delete_hub would escape the read-only clamps"); +}); + +test("the other workspace-destroying services are deliberately NOT aligned", () => { + // merge_workspace empties a whole workspace and copy_workspace duplicates + // one; both still ask owner. They were left alone on purpose — moving them is + // a decision of its own, not a tidy-up. See their `doc` in acl/media.json. + const media = ACL("media"); + assert.equal(media.merge_workspace.permission.src, "owner"); + assert.equal(media.copy_workspace.permission.src, "owner"); +}); + +test("a personal workspace can never come through delete_hub", () => { + // service/private/hub.js refuses anything whose entity type is not `hub`, so + // a personal workspace (a folder in the caller's own home, whose hub_id is + // the caller's drumate) meets WRONG_ENTITY_TYPE rather than this permission. + const src = readFileSync( + join(__dirname, "..", "..", "service", "private", "hub.js"), "utf8", + ); + const body = src.slice(src.indexOf("async delete_hub()")); + const guard = body.indexOf("WRONG_ENTITY_TYPE"); + const destroy = body.indexOf("entity_delete"); + assert.ok(guard > 0, "delete_hub lost its entity-type guard"); + assert.ok(destroy > guard, "entity_delete is reachable before the type guard"); +}); From 0ee722c7ee27f68487cd23ceb1cbc5a80dfe19d5 Mon Sep 17 00:00:00 2001 From: EddyOne81 Date: Thu, 17 Sep 2026 01:32:54 -0700 Subject: [PATCH 11/15] fix(test): the delete_hub permission test must run without node_modules MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The CI job it was just wired into (.github/workflows/test.yml "Test - secure-share regression") deliberately does NOT `npm install` — it runs on a stock GitHub runner with no private-registry auth, so @drumee/server-essentials is absent. Requiring it at module scope would have thrown MODULE_NOT_FOUND and turned the step red on the next PR into a mainline branch, which is where that workflow actually fires. So the bit values are declared locally and every assertion works from them; when the package IS present an extra case checks the local copy still matches the shipped table, so the constants cannot drift unnoticed either. Same arrangement secure-share-session.test.js uses, and for the same reason. Verified both ways: 6 pass with node_modules, 5 pass + 1 skipped in a bare `git archive` checkout with none, and still RED on a src reverted to owner. Co-Authored-By: Claude Opus 5 (1M context) --- offline/test/hub-delete-permission.test.js | 48 ++++++++++++++-------- 1 file changed, 31 insertions(+), 17 deletions(-) diff --git a/offline/test/hub-delete-permission.test.js b/offline/test/hub-delete-permission.test.js index c974b62f..cfb43140 100644 --- a/offline/test/hub-delete-permission.test.js +++ b/offline/test/hub-delete-permission.test.js @@ -37,38 +37,55 @@ * (router/rest/index.js `mightMutate`). Owner and admin both clear that * bar; anything at or below `read` would not. * - * The bitmask values come from the INSTALLED @drumee/server-essentials, not - * from a copy, so a change to the shared table is a failure here rather than a - * silent drift. + * DEPENDENCY-FREE BY DESIGN. The CI job that runs this deliberately does not + * `npm install` (see .github/workflows/test.yml), so the private + * @drumee/server-essentials package is usually absent on the runner. The bit + * values are therefore declared locally and every assertion works from them; + * when the package IS present we additionally check the local copy still + * matches the shipped table, so drift is caught rather than assumed away. Same + * arrangement as secure-share-session.test.js. */ const test = require("node:test"); const assert = require("node:assert"); const { readFileSync } = require("node:fs"); const { join } = require("node:path"); -const { permissionValue } = require("@drumee/server-essentials"); - +const REPO_ROOT = join(__dirname, "..", ".."); const ACL = (name) => - JSON.parse(readFileSync(join(__dirname, "..", "..", "acl", `${name}.json`), "utf8")) - .services; + JSON.parse(readFileSync(join(REPO_ROOT, "acl", `${name}.json`), "utf8")).services; + +// server-essentials lib/lex/permission.js — the single BITS a service asks for. +const BIT = { read: 0b0000010, write: 0b0001000, admin: 0b0010000, owner: 0b0100000 }; // The stored privilege WORDS a member can hold — hub.set_privilege writes these -// and user_permission() hands them back. Kept literal on purpose: they are the -// thing under test, so deriving them from the same table would prove nothing. +// and user_permission() hands them back. const ROLE = { view: 0b0000011, chat: 0b0000111, edit: 0b0001111, admin: 0b0011111, owner: 0b0111111 }; // Exactly what lib/acl.js check_source does with each row. const granted = (privilege, asked) => !!(privilege & asked); +let permissionValue = null; +try { + ({ permissionValue } = require("@drumee/server-essentials")); +} catch (e) { + console.log(` ~ shipped-table cross-check SKIPPED (server-essentials not installed: ${e.code || e.message})`); +} + +test("the local bit values still match the shipped table", { skip: !permissionValue }, () => { + for (const [name, value] of Object.entries(BIT)) { + assert.equal(permissionValue(name), value, `permission bit \`${name}\` drifted`); + } +}); + test("hub.delete_hub asks for the admin bit", () => { const spec = ACL("hub").delete_hub; assert.equal(spec.permission.src, "admin"); assert.equal(spec.scope, "hub"); - assert.equal(permissionValue(spec.permission.src), 0b0010000); }); test("Admin and Owner may delete; Edit, Chat and View may not", () => { - const asked = permissionValue(ACL("hub").delete_hub.permission.src); + const asked = BIT[ACL("hub").delete_hub.permission.src]; + assert.ok(asked, "delete_hub asks for a bit this test does not know"); for (const [role, want] of Object.entries({ view: false, chat: false, edit: false, admin: true, owner: true, })) { @@ -84,9 +101,8 @@ test("delete_hub still sits above `read`, so the read-only clamps still catch it // secure-share recipient carries a read-only session ceiling and an // over-limit domain is clamped to reads; both rely on this comparison, NOT on // a service name, so widening owner -> admin must not cross that line. - const READ = permissionValue("read"); - const src = permissionValue(ACL("hub").delete_hub.permission.src); - assert.ok(src > READ, "delete_hub would escape the read-only clamps"); + const src = BIT[ACL("hub").delete_hub.permission.src]; + assert.ok(src > BIT.read, "delete_hub would escape the read-only clamps"); }); test("the other workspace-destroying services are deliberately NOT aligned", () => { @@ -102,9 +118,7 @@ test("a personal workspace can never come through delete_hub", () => { // service/private/hub.js refuses anything whose entity type is not `hub`, so // a personal workspace (a folder in the caller's own home, whose hub_id is // the caller's drumate) meets WRONG_ENTITY_TYPE rather than this permission. - const src = readFileSync( - join(__dirname, "..", "..", "service", "private", "hub.js"), "utf8", - ); + const src = readFileSync(join(REPO_ROOT, "service", "private", "hub.js"), "utf8"); const body = src.slice(src.indexOf("async delete_hub()")); const guard = body.indexOf("WRONG_ENTITY_TYPE"); const destroy = body.indexOf("entity_delete"); From 5879cfafbf856afb864205d2353fba50a3b8b305 Mon Sep 17 00:00:00 2001 From: Aaron Vu Date: Thu, 17 Sep 2026 15:33:25 +0700 Subject: [PATCH 12/15] fix(hub): grant chat staging write by role, and take it back on demotion The three call sites that give a new member access to the hidden chat staging folder granted a fixed value of 4. That value meant write before the permission bits were renumbered and means download now, so a member whose role carries no write bit could never upload an attachment and got a bare 403 with nothing in the UI to explain it. Grant write instead, but only to a role that may chat. The grant was previously unconditional, so a view-only member already carried it; left unconditional it would have handed them an upload path once the value became meaningful. set_privilege also has to move the grant in both directions. It only ever granted, so demoting a member from chat to view left the staging access behind and the demoted member kept uploading. Revoke it there instead. --- service/private/hub.js | 52 +++++++++++++++++++++++++++++------------- 1 file changed, 36 insertions(+), 16 deletions(-) diff --git a/service/private/hub.js b/service/private/hub.js index 5f1af3a3..58dfa117 100644 --- a/service/private/hub.js +++ b/service/private/hub.js @@ -33,6 +33,7 @@ const { butlerFrom } = require("../lib/mail-sender"); const { mailFailure } = require("../lib/mail-result"); const { resolveHubInviteName } = require("../lib/hub-invite-name"); const { resolveHubDisplayName } = require("../lib/hub-display-name"); +const { CAN_CHAT, privilegeAllows } = require("../lib/member-capability"); const { MfsTools } = require("@drumee/server-core"); const { remove_dir } = MfsTools; const { toArray } = utils; @@ -1001,10 +1002,17 @@ class __private_hub extends Hub { await this.db.await_proc( "permission_grant", "*", uid, expiry, privilege, "system", message ); - await this.db.await_proc( - "permission_grant", mfs_home.chat_upload_id, uid, 0, 4, - "no_traversal", "chat upload permission" - ); + // Chat attachments stage in a hidden folder before they become a message. + // A member who may chat has to write there even though the role carries no + // write bit for the workspace at large -- 'no_traversal' keeps that raised + // access on this one folder. A view-only member may not chat, so granting + // it would hand them an upload path they are not entitled to. + if (privilegeAllows(privilege, CAN_CHAT)) { + await this.db.await_proc( + "permission_grant", mfs_home.chat_upload_id, uid, 0, Privilege.WRITE, + "no_traversal", "chat upload permission" + ); + } await writeAudit(this, { db: this.hub.get(Attr.db_name), uid: this.uid, @@ -2139,14 +2147,16 @@ class __private_hub extends Hub { '*', uid, expiry, privilege, 'system', msg ); - // Grant chat upload permission if chat folder exists - if (mfs_home && mfs_home.chat_upload_id) { + // Write access to the chat staging folder only, and only for a role + // that may chat -- see _grantMembership for why it is scoped this way. + if (mfs_home && mfs_home.chat_upload_id && + privilegeAllows(privilege, CAN_CHAT)) { await this.yp.await_proc( `${hub_db}.permission_grant`, mfs_home.chat_upload_id, uid, 0, // no expiry on chat upload - 4, // read+write for uploads + Privilege.WRITE, 'no_traversal', 'chat upload permission' ); @@ -2812,15 +2822,25 @@ class __private_hub extends Hub { for (let uid of users) { await this.db.await_proc("permission_set", uid, privilege); - await this.db.await_proc( - "permission_grant", - mfs_home.chat_upload_id, - uid, - 0, - 4, - "no_traversal", - "chat upload permission" - ); + // A role change has to move the chat staging grant in BOTH directions. + // Granting on the way up is what lets a chat member attach a file; + // revoking on the way down is what stops a member demoted to view-only + // from keeping an upload path the new role does not carry. + if (privilegeAllows(privilege, CAN_CHAT)) { + await this.db.await_proc( + "permission_grant", + mfs_home.chat_upload_id, + uid, + 0, + Privilege.WRITE, + "no_traversal", + "chat upload permission" + ); + } else { + await this.db.await_proc( + "permission_revoke", mfs_home.chat_upload_id, uid + ); + } hub = {}; hub.privilege = privilege; From 8d6f7bb8a1696e43e2df30c74735848ff1cbf2a7 Mon Sep 17 00:00:00 2001 From: Aaron Vu Date: Thu, 17 Sep 2026 15:49:21 +0700 Subject: [PATCH 13/15] fix(hub): pin the chat staging grant value instead of reading it from the package Privilege.WRITE resolves to 15 under server-essentials 1.3.1 and to 7 under 1.3.6, which republished the pre-1.3.0 bit layout. package.json asks for ^1.3.1, so a bare npm install picks up 1.3.6 and the grant would carry no write bit at all -- the exact 403 this change exists to fix, reintroduced by a dependency bump nobody would connect to chat. Pin the value alongside the other capability bits, which are spelled out for the same reason and say so in the file header. --- service/lib/member-capability.js | 19 +++++++++++++++++++ service/private/hub.js | 10 ++++++---- 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/service/lib/member-capability.js b/service/lib/member-capability.js index d7231656..087cd925 100644 --- a/service/lib/member-capability.js +++ b/service/lib/member-capability.js @@ -41,6 +41,24 @@ const CAN_CHAT = 0b0000100; const CAN_WRITE = 0b0001000; const CAN_ADMIN = 0b0010000; +/** + * What a member is granted on the hidden chat staging folder + * ('/__chat__/__upload__'), where an attachment is held before it becomes a + * message. read + download + write, plus the low bit every stored role mask + * carries. The grant is written with assign_via 'no_traversal', so it applies + * to that one folder and user_permission will not let it reach anything + * inside it. + * + * 🚨 Pinned here for the same reason every other value in this file is, and + * this one has already moved twice. `Privilege.WRITE` resolves to 15 under + * server-essentials 1.3.1 and to 7 under 1.3.6, which republished the + * pre-1.3.0 layout. package.json asks for ^1.3.1, so a bare `npm install` + * picks up 1.3.6 and a grant written from `Privilege.WRITE` would carry no + * write bit at all -- the exact 403 this value exists to fix, reintroduced + * by a dependency bump nobody connected to chat. + */ +const CHAT_UPLOAD_GRANT = 0b0001111; + /** * Does this stored privilege carry every bit of `bit`? * @@ -145,6 +163,7 @@ async function memberCan(service, bit) { module.exports = { CAN_READ, + CHAT_UPLOAD_GRANT, CAN_DOWNLOAD, CAN_CHAT, CAN_WRITE, diff --git a/service/private/hub.js b/service/private/hub.js index 58dfa117..24563ef7 100644 --- a/service/private/hub.js +++ b/service/private/hub.js @@ -33,7 +33,9 @@ const { butlerFrom } = require("../lib/mail-sender"); const { mailFailure } = require("../lib/mail-result"); const { resolveHubInviteName } = require("../lib/hub-invite-name"); const { resolveHubDisplayName } = require("../lib/hub-display-name"); -const { CAN_CHAT, privilegeAllows } = require("../lib/member-capability"); +const { + CAN_CHAT, CHAT_UPLOAD_GRANT, privilegeAllows +} = require("../lib/member-capability"); const { MfsTools } = require("@drumee/server-core"); const { remove_dir } = MfsTools; const { toArray } = utils; @@ -1009,7 +1011,7 @@ class __private_hub extends Hub { // it would hand them an upload path they are not entitled to. if (privilegeAllows(privilege, CAN_CHAT)) { await this.db.await_proc( - "permission_grant", mfs_home.chat_upload_id, uid, 0, Privilege.WRITE, + "permission_grant", mfs_home.chat_upload_id, uid, 0, CHAT_UPLOAD_GRANT, "no_traversal", "chat upload permission" ); } @@ -2156,7 +2158,7 @@ class __private_hub extends Hub { mfs_home.chat_upload_id, uid, 0, // no expiry on chat upload - Privilege.WRITE, + CHAT_UPLOAD_GRANT, 'no_traversal', 'chat upload permission' ); @@ -2832,7 +2834,7 @@ class __private_hub extends Hub { mfs_home.chat_upload_id, uid, 0, - Privilege.WRITE, + CHAT_UPLOAD_GRANT, "no_traversal", "chat upload permission" ); From 2662f048235ca7ba044018cdd1efa404392c198a Mon Sep 17 00:00:00 2001 From: Aaron Vu Date: Thu, 17 Sep 2026 17:00:00 +0700 Subject: [PATCH 14/15] test(hub): lock who may write to the chat staging folder, and with what value MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both properties the attachment fix rests on can be undone by an edit that reads like a simplification, and neither shows up as a test failure today. The gate must refuse a view-only member. The grant went out regardless of role before, so rows for view-only members already exist in the wild; loosening the check to a partial bit overlap would admit exactly the role it exists to exclude, because the chat constant in the package overlaps read. The granted value must carry the write bit and must not be read from the package. server-essentials 1.3.6 republished the pre-1.3.0 layout, where Privilege.WRITE is 7 and carries no write bit in this schema, and package.json asks for ^1.3.1 — so a dependency bump nobody connects to chat would silently restore the 403. Reads the call sites as text rather than loading them, and imports only member-capability, which requires nothing. That keeps it in the same install-free workflow as the other regressions here. Checked against three mutations: restoring the package value, dropping the gate from one call site, and removing the revoke on demotion. Each fails the suite. --- .github/workflows/test.yml | 6 ++ offline/test/chat-upload-grant.test.js | 139 +++++++++++++++++++++++++ 2 files changed, 145 insertions(+) create mode 100644 offline/test/chat-upload-grant.test.js diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 4bcb3411..477f3acc 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -47,3 +47,9 @@ jobs: # the over-limit clamp keep catching it. - name: Run workspace-delete permission regression test run: node --test offline/test/hub-delete-permission.test.js + # The other permission bar with a CI step of its own: it pins that a + # view-only member gets no write on the chat staging folder, and that the + # granted value keeps the write bit whichever server-essentials is + # installed. + - name: Run chat staging grant regression test + run: node --test offline/test/chat-upload-grant.test.js diff --git a/offline/test/chat-upload-grant.test.js b/offline/test/chat-upload-grant.test.js new file mode 100644 index 00000000..85713104 --- /dev/null +++ b/offline/test/chat-upload-grant.test.js @@ -0,0 +1,139 @@ +#!/usr/bin/env node + +// Who may write to the chat staging folder, and with what value. +// +// THE REPORT: a member whose workspace role is Chat could not attach a file in +// chat. The upload answered 403 and the chat showed nothing at all -- no +// attachment chip, no error -- while the same account attached files normally +// in a workspace where it happened to be an admin. +// +// An attachment stages in the hidden folder '/__chat__/__upload__' before it +// becomes a message. A chat member has no write bit for the workspace, which is +// intended, so the membership paths give write on that one folder through a +// grant written with assign_via 'no_traversal'. Two things were wrong: the +// value granted was 4, which meant write before the permission bits were +// renumbered and means download now, and the grant went out regardless of role, +// so view-only members held it too. +// +// These lock the two properties the fix rests on, both of which can be undone +// by an edit that looks like a simplification: +// +// 1. the gate admits a role that may chat and refuses one that may not; +// 2. the granted value carries the write bit, and does not come from the +// package, whose 1.3.6 release republished the pre-1.3.0 layout. +// +// Deliberately dependency-free: member-capability.js requires nothing, and the +// call sites are read as text, so this runs on a stock runner with no install. + +const test = require("node:test"); +const assert = require("node:assert/strict"); +const { readFileSync } = require("node:fs"); +const { join } = require("node:path"); + +const { + CAN_CHAT, + CAN_WRITE, + CAN_READ, + CHAT_UPLOAD_GRANT, + privilegeAllows, +} = require("../../service/lib/member-capability"); + +// The stored privilege for each role selector the UI offers. +const VIEW = 0b000011; +const CHAT = 0b000111; +const EDIT = 0b001111; +const ADMIN = 0b011111; +const OWNER = 0b111111; + +test("the gate refuses a view-only member", () => { + assert.equal(privilegeAllows(VIEW, CAN_CHAT), false, + "a member who may only read must not be given write on the chat staging " + + "folder; the grant used to go out regardless of role, so rows for " + + "view-only members already exist in the wild"); +}); + +test("the gate admits every role at or above chat", () => { + for (const [name, privilege] of [ + ["chat", CHAT], ["edit", EDIT], ["admin", ADMIN], ["owner", OWNER], + ]) { + assert.equal(privilegeAllows(privilege, CAN_CHAT), true, + `${name} may chat and must be able to attach a file`); + } +}); + +// The trap the gate exists to avoid. `Constants.permission.chat` is 0b0000110 -- +// read OR download -- so it OVERLAPS read, and a loose `privilege & chat` is +// truthy for a view-only member (3 & 6 = 2). privilegeAllows asks for every bit +// of the mask, so the overlap is not enough. +test("a multi-bit mask that overlaps read still refuses a view-only member", () => { + const READ_OR_DOWNLOAD = 0b0000110; + assert.notEqual(VIEW & READ_OR_DOWNLOAD, 0, + "the premise: the loose form would admit a view-only member here"); + assert.equal(privilegeAllows(VIEW, READ_OR_DOWNLOAD), false, + "privilegeAllows must require the whole mask, not a partial overlap"); +}); + +test("the granted value carries the write bit", () => { + assert.equal((CHAT_UPLOAD_GRANT & CAN_WRITE) === CAN_WRITE, true, + "without the write bit the upload ACL refuses and the member gets the " + + "same silent 403 this change exists to fix"); + assert.equal((CHAT_UPLOAD_GRANT & CAN_READ) === CAN_READ, true, + "the member has to read the staging folder as well as write to it"); +}); + +// server-essentials 1.3.6 republished the pre-1.3.0 layout: Privilege.WRITE +// resolves to 7 there and to 15 under the pinned 1.3.1. package.json asks for +// ^1.3.1, so reading the value from the package means a dependency bump nobody +// connects to chat silently removes the write bit again. +test("the granted value is not the value the republished package would give", () => { + assert.notEqual(CHAT_UPLOAD_GRANT, 0b0000111, + "0b0000111 is Privilege.WRITE under server-essentials 1.3.6 and carries " + + "no write bit in this schema"); +}); + +// The gate has to sit at the call site. A check one layer up is not the same +// invariant: three separate paths grant this row, and each has its own idea of +// where `privilege` comes from. +const HUB_SOURCE = readFileSync( + join(__dirname, "..", "..", "service", "private", "hub.js"), + "utf8" +); + +test("every chat staging grant in hub.js is gated at the call site", () => { + const lines = HUB_SOURCE.split("\n"); + const sites = []; + lines.forEach((line, i) => { + if (line.includes("chat_upload_id") && !line.trim().startsWith("*")) { + sites.push(i); + } + }); + assert.ok(sites.length >= 3, + `expected the three membership paths to grant this row, found ${sites.length}`); + + for (const i of sites) { + const window = lines.slice(Math.max(0, i - 12), i + 2).join("\n"); + const isGrant = window.includes("permission_grant"); + if (!isGrant) continue; + assert.ok( + window.includes("CAN_CHAT") && window.includes("privilegeAllows"), + `the permission_grant near line ${i + 1} writes the chat staging row ` + + "without a chat-bit check above it" + ); + assert.ok( + !/,\s*4\s*,/.test(window), + `the permission_grant near line ${i + 1} still passes a bare 4, the ` + + "value that means download since the bits were renumbered" + ); + } +}); + +test("a role change gives the row back and takes it away", () => { + const start = HUB_SOURCE.indexOf("async set_privilege()"); + assert.notEqual(start, -1, "set_privilege not found in hub.js"); + const body = HUB_SOURCE.slice(start, start + 2000); + assert.ok(body.includes("permission_grant"), + "promoting a member to a chat role has to give them the staging row"); + assert.ok(body.includes("permission_revoke"), + "demoting a member to view-only has to take the staging row back; " + + "set_privilege only ever granted, so a demoted member kept uploading"); +}); From a631d58a6c80556e17eed5b525b225ca93173902 Mon Sep 17 00:00:00 2001 From: Tran Hoang Huan <121786621+tranh0anghuan@users.noreply.github.com> Date: Fri, 18 Sep 2026 23:03:04 +0700 Subject: [PATCH 15/15] fix(warmup): the boot screen fits the device it boots on (#224) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Measured in a headless browser at 320 through 1440: the splash overflowed by 8px across and 16px down at EVERY viewport. still carries the UA's default 8px margin while this screen is up — loader.css ships no reset and the app's stylesheet has not arrived yet, which is the whole reason the screen exists — so `width:100vw; height:100vh` never fit. On a phone that is a splash you can pan sideways with its right edge cut off. The card was worse: 455px of content plus 32px padding is a 519px border box, and loader.css caps it with max-width:100vw, which caps the CONTENT box. On a 375px phone it measured 439px at x:-24, hanging off both edges with no effective padding, and the progress track — left:0/right:0 on that card — ran the full width of the screen with its rounded ends clipped off. - position:fixed resolves against the initial containing block, so the body margin cannot reach the splash and no ancestor can resize it - 100dvh after 100vh: 100vh on a phone is the viewport with the URL bar retracted, so centring against it put the lockup low, behind the toolbar, for as long as the toolbar showed - border-box on the card, with its 519px footprint restated as a sum so nothing above phone width is resized - phone, tablet and short-viewport breakpoints: full bleed (the 32px radius is drawn on a full-screen element, so it notches the display corners rather than rounding a card), compacted padding, lockup and caption Overflow is now 0 in both axes from 280px to 1440px. Above tablet width the card, lockup and bar keep their exact measurements; the only change there is that the screen is no longer offset by the body margin. Co-authored-by: Drumee Dev Co-authored-by: Claude Opus 5 (1M context) --- client/templates/warmup.html | 136 +++++++++++++++++++++++++++++++++++ 1 file changed, 136 insertions(+) diff --git a/client/templates/warmup.html b/client/templates/warmup.html index 5331cd73..e0b1fe9a 100644 --- a/client/templates/warmup.html +++ b/client/templates/warmup.html @@ -63,6 +63,142 @@ .warmup-progress { padding-bottom: 12px; } + + /* ========================================================================= + RESPONSIVE + Measured before writing any of this, with loader.css + this template in a + headless Chromium at 320/375/390/740x360/768/834/1024/1440. Every number + quoted below is from that run. + ========================================================================= */ + + /* The splash is the viewport, not a 100vw x 100vh box inside a margined body. + At boot still carries the UA's default 8px margin — loader.css ships + no reset and the app's stylesheet has not arrived yet, which is the entire + reason this screen is on display. So loader.css:380's `width:100vw; + height:100vh` overflowed by 8px across and 16px down at EVERY viewport + measured: a 375px phone scrolled 383x683. On a handset that is a splash + screen you can pan sideways, with the card's right edge cut off. + + position:fixed resolves against the initial containing block, so the body + margin cannot reach it and no ancestor can resize it — one property instead + of a calc() that would have to name the 8px margin and track it. + + 100dvh AFTER 100vh, both declared: 100vh on a phone is the LARGEST viewport + (URL bar retracted), so centring against it puts the lockup low, partly + behind the toolbar, for as long as the toolbar is showing — which on a boot + screen is the whole time. dvh tracks the bar. The 100vh line stays as the + fallback for engines that drop the dvh declaration. */ + .warmup-main { + position: fixed; + top: 0; + left: 0; + width: 100%; + height: 100vh; + height: 100dvh; + } + + /* The card must be a ceiling, not a floor. loader.css:405 sets width:455px on + a content-box with 32px padding — a 519px border box — and caps it with + max-width:100vw, which caps the CONTENT box, so the border box stayed 64px + wider than the screen. Measured on a 375px phone: the card was 439px at + x:-24, hanging 24px off BOTH edges, with zero effective padding. The + progress track (::after above) is `left:0; right:0` on that card, so it ran + the full width of the screen and had its 100px rounded ends clipped off at + each side. + + border-box makes the declared width include the padding, so max-width:100% + clamps the whole box. 100% rather than another 100vw: on desktop 100vw + includes the scrollbar gutter, 100% does not. + + The width is restated as calc(455px + 64px) because switching to border-box + would otherwise have RESHAPED the desktop card: loader.css's 455px would + start including the padding, leaving 391px of content and a 391px progress + bar where the design has always had 455px. Written as a sum rather than as + 519px so it stays legible as "loader.css:405's width, plus the padding a + side it was always drawn outside of". Nothing above phone width is resized + by this change. */ + .warmup-01 { + box-sizing: border-box; + width: calc(455px + 64px); + max-width: 100%; + } + + /* --- Phone, portrait ---------------------------------------------------- */ + @media (max-width: 599px) { + /* loader.css:385 rounds .warmup-main by 32px, but that element is the whole + screen — on a handset the radius does not round a card, it punches four + notches of the body's #f6f6f6 into the corners of the display. */ + .warmup-main { + border-radius: 0; + } + + /* 32px a side spends 64px of a 320px screen on air. 20px keeps a visible + gutter while leaving the progress bar a run long enough to read as a + progress bar. */ + .warmup-01 { + padding: 24px 20px; + gap: 12px; + } + + /* 72vw was sized for the desktop card, where it never binds (it is 240px + from 334px up). On a phone it DOES bind, and 72vw of a 320px screen is + 230px of lockup — the mark ends up louder than the app it is booting. + 60vw holds it to ~192px there and still hits the designed 240px by 400px + wide. */ + .warmup-symbol svg.warmup-logo { + width: min(240px, 60vw); + } + + /* 20px/30px is the desktop caption. At phone width it sits directly over a + bar that is now ~335px instead of 455px; 17px keeps "Warming Up..." on + one line at 320px with the shorter measure. */ + .warmup-progress-text { + font-size: 17px; + line-height: 24px; + } + } + + /* --- Tablet ------------------------------------------------------------- */ + /* The 519px card is already right on a 768-1024px screen, so this block does + not resize it — the lockup deliberately keeps its designed 240px footprint + here rather than being inflated to fill a bigger display. What a tablet + needs is the same full-bleed correction as a phone (it is a device screen, + not a window, so the same four corner notches apply) and a guaranteed + gutter so the card never grows flush to the edges on the narrow end of the + range. */ + @media (min-width: 600px) and (max-width: 1024px) { + .warmup-main { + border-radius: 0; + } + + .warmup-01 { + max-width: calc(100% - 64px); + } + } + + /* --- Short viewports: phone and tablet in landscape --------------------- */ + /* Keyed on height, not on a device: a 740x360 phone held sideways and a + tablet with the keyboard up both land here. The screen is centred content + in a box roughly 300px tall once the browser chrome is out, and the + portrait stack (240px lockup + 16px gap + 42px progress row + 64px padding) + is most of that. Compacting the lockup and the padding keeps the caption + and the bar clear of the chrome instead of being clipped by + .warmup-main's overflow:hidden. */ + @media (max-height: 480px) { + .warmup-01 { + padding: 16px 20px; + gap: 8px; + } + + .warmup-symbol svg.warmup-logo { + width: min(180px, 34vw); + } + + .warmup-progress-text { + font-size: 16px; + line-height: 22px; + } + }