Conversation
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) <noreply@anthropic.com>
… 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) <noreply@anthropic.com>
`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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
… 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) <noreply@anthropic.com>
hub.add_contributors and hub.invite_with_roles mailed "<inviter> 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) <noreply@anthropic.com>
…d click 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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
The boot screen drew the cloud mark as inline SVG beside a <span>drumee</span> 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 <img src>: 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 <style> block rather than in loader.css, which is not in this repo (/home/drumee/static/styles/loader.css, deployed to /srv/drumee/static). The block is later in document order at the same specificity, so it wins without a second edit in a second place. Notably loader.css pins `.warmup-symbol svg` to width:64px, which would squash the lockup to 64x12.7. The export's preserveAspectRatio="none" is dropped so the viewBox drives the height and an off-ratio box can never stretch the mark. The caption goes to #0B0A21 — the lockup's own black, not #000, so the two lines read as one colour — and the bar sits 12px under it instead of 5px, via padding on the row so the track (::after) and the fill move together. Co-authored-by: Drumee Dev <drumee@debian.local.drumee> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
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.
… 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.
…at value 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.
fix(hub): grant chat staging write by role, and take it back on demotion
Measured in a headless browser at 320 through 1440: the splash overflowed by 8px across and 16px down at EVERY viewport. <body> 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 <drumee@debian.local.drumee> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
# Conflicts: # service/private/hub.js
|
|
Claude encountered an error after 0s —— View job I'll analyze this and get back to you. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 79dbd73d6b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Comment on lines
+2841
to
+2844
| } else { | ||
| await this.db.await_proc( | ||
| "permission_revoke", mfs_home.chat_upload_id, uid | ||
| ); |
There was a problem hiding this comment.
When an admin demotes a chat/edit/admin member through the separately exposed hub.set_member_privilege service, that method replaces only the '*' grant and never reaches this revoke branch. Because this change now gives chat-capable members a node-level CHAT_UPLOAD_GRANT containing write, a user demoted through that endpoint retains permanent write access to the staging folder and can keep uploading as view-only; apply the same staging grant/revoke synchronization in set_member_privilege.
Useful? React with 👍 / 👎.
Comment on lines
+1013
to
+1015
| await this.db.await_proc( | ||
| "permission_grant", mfs_home.chat_upload_id, uid, 0, CHAT_UPLOAD_GRANT, | ||
| "no_traversal", "chat upload permission" |
There was a problem hiding this comment.
When add_contributors supplies nonzero days or hours, _grantMembership applies that expiry to the workspace grant but hard-codes 0 (no expiry) on this new write-capable node grant. After the membership expires, the direct staging permission remains and can still satisfy the write check for uploads to that folder, leaving a former member able to consume workspace storage; use the membership expiry here as well.
Useful? React with 👍 / 👎.
Comment on lines
+1012
to
+1015
| if (privilegeAllows(privilege, CAN_CHAT)) { | ||
| await this.db.await_proc( | ||
| "permission_grant", mfs_home.chat_upload_id, uid, 0, CHAT_UPLOAD_GRANT, | ||
| "no_traversal", "chat upload permission" |
There was a problem hiding this comment.
When a Chat user joins by redeeming an emailed token or by signing up from a pending invitation, the repository's accept_invite, signup._resolve_pending_invitation, and butler._resolve_pending_invitation paths call add_member and grant only '*'; they never pass through this new staging grant. Those common invitation flows therefore still produce Chat members whose attachment uploads receive the original 403, so the role-aware grant needs to be centralized or added to those paths too.
Useful? React with 👍 / 👎.
| * 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; |
There was a problem hiding this comment.
For every Chat member created before this deployment, the persisted staging row still contains the old value 4; defining and using CHAT_UPLOAD_GRANT only affects later invitations or explicit role changes. No migration or repository backfill updates those existing rows, so the production users whose attachment uploads motivated this change continue receiving 403 until an admin happens to rewrite their role; reconcile existing grants during rollout.
Useful? React with 👍 / 👎.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Whole-branch promotion
test→preview, requested by Natrix.Diff vs
preview: 8 files, +542, −25.Payload
acl/hub.jsondelete_hub) + its regression test. The owner bar was never a real boundary —hub.change_ownerandhub.set_privilegeare admin-gated and unclamped — while the client already offered a Delete row gated on the admin bit, so an admin got a 403. The ui half is already onpreview, so this closes that mismatch./__chat__/__upload__, a demotion revokes it, and the granted value is pinned asCHAT_UPLOAD_GRANT = 0b0001111instead of read fromPrivilege.WRITE(which resolves to 15 under server-essentials 1.3.1 and 7 under 1.3.6).merge_workspacedoc note: it deliberately stays at owner and is not to be aligned withdelete_hubwithout its own decision.preview(cherry-picked there earlier, no net change here): the invite "invited you to <workspace>" wording, the invite mail/audit workspace name, themedia.copyfolder chip, and Lương's cross-domain invite commit + its revert.Back-merge note
preview→testconflicted onservice/private/hub.jsonly. Resolved totest: every preview-only line was the old inlinepermission_grant(..., 4, ...)that #222 replaced with the role-gatedCHAT_UPLOAD_GRANTpath. After resolving,git diff --stat origin/testis empty — the merge carries no content of its own.🤖 Generated with Claude Code