With multiple accounts and settings sync on, the composer's default sender
would switch depending on login order and get disturbed by logging a third
account off. Cause: the per-account maps preferredIdentityIds (#507) and
allMailFolderIds live in EVERY account's synced settings blob, and
loadFromServer(username, serverUrl) runs once per connected account calling
importSettings(), which REPLACED the whole map with that account's server copy.
So the last account to load clobbered the map (last-writer-wins by login
order), and a stale blob from a since-removed account could win too.
Fix: importSettings takes an optional { serverAccountId }; loadFromServer passes
generateAccountId(username, serverUrl). For the per-account maps, a server load
now merges only that account's OWN entry into the existing map instead of
replacing it - each account's server is authoritative only for itself, so other
accounts' entries survive. File imports (no serverAccountId) still replace
wholesale.
+ regression tests: a per-account load preserves other accounts' entries,
only trusts the loading account, file import still replaces, same for
allMailFolderIds. tsc + eslint clean.
Companion to the calendar spec for the cross-account id-namespacing fix. Logs
in two accounts, enables the Pro shell, and asserts both accounts' address books
stay aggregated (same set, no loss/duplication/corruption) after switching the
active account. Contacts keep no persisted id-selection, so this verifies the
aggregated list stays correct/usable rather than a selection reset.
Adds a data-testid on address-book rows (address-book-item + data-book-name/
-account) and openContacts / addressBookRows helpers.
Docker/Playwright regression for the cross-account id-namespacing fix: log in
two accounts, enable the Pro shell, deselect a calendar, switch the active
account, and assert the deselection survives (it reset before the fix because
the previously-active account's ids flipped raw<->namespaced on the switch).
Adds the minimal test hooks: data-testid on calendar sidebar rows
(calendar-item + data-calendar-name/-visible), the settings tab buttons
(settings-tab-<id>), and a testId passthrough on ToggleSwitch used for the Pro
interface toggle. New app helpers: openCalendar (in-app nav, since a hard goto
to a deep link bounces to login), calendarRows, enableProInterface (toggles via
the settings UI - the app doesn't survive a hard reload and seeding proInterface
before login destabilises the add-account popover).
Verified green in the dockerized Stalwart harness.
Calendar/contact aggregation left the ACTIVE account's ids raw while
namespacing background accounts (<localAccountId>::id). So an entity's id
depended on which account was active; switching flipped the id form and
invalidated persisted references (selectedCalendarIds), broke isSubscription
matching, and destabilised selection - the account-switch bug users saw in
Calendar and Address Book (mail was fine: it keys per account + stable
virtual ids).
Invariant now: an entity is namespaced iff it carries a localAccountId;
originalId always holds the raw JMAP id. Consumers/mutations already resolve
the raw id via `originalId || stripLocalAccountPrefix`, so they are unchanged.
- prefix* functions (calendar + contacts) always namespace + set originalId;
drop the isActiveAccount branch and the now-dead activeLocalAccountId params
(callers in the hooks + calendar-app/contacts-app updated).
- mapServerEventToStoreEvent namespaces the event id with the matched calendar's
localAccountId, so createEvent/updateEvent results are consistent.
- reconcileSelectedIds(): remap selectedCalendarIds by raw identity on every
fetch, so a raw->namespaced transition (switch, just-created calendar, or the
one-time migration from old persisted raw ids) keeps the selection instead of
resetting to "all".
- isSubscriptionCalendar(): namespace-robust and collision-safe - resolve the
calendar and match by raw id scoped to the owning JMAP accountId (Stalwart's
raw ids like "b"/"c" collide across accounts).
+9 unit tests (selection stability across switch/create, subscription
robustness, no cross-account false positives). tsc + eslint clean; 368 store
tests green.
The rights-first resolver already made EXISTING events in subscription /
read-only calendars read-only, but the create path still offered them:
event-modal's calendar dropdown listed every calendar. iCal subscriptions are
real Stalwart calendars (client-side feed written into a created calendar) with
writable myRights, so they appeared as selectable create destinations.
- canCreateEventsIn(calendar, isSubscriptionCalendar): writable AND not a
subscription. Exported from lib/calendar-editability.
- event-modal: the calendar picker now lists only creatable calendars (plus the
event's own calendar when editing), and new events never default into a
subscription / read-only calendar.
+4 unit tests. tsc + eslint clean; calendar-editability suite 30 green.
Event editability was inferred from identity (isOrigin + organizer match)
and never from the calendar's write rights. That opened events in read-only
shared / iCal-subscription calendars for editing, and once the address list
was widened with account aliases any alias-organized event became editable -
even in calendars the user cannot write.
- lib/calendar-editability.ts: getEventEditability() -> editable | rsvp-only
| read-only, gating on the calendar's myRights (mayWriteAll / mayWriteOwn /
mayRSVP) first, then organizer/participant identity. Subscriptions are
read-only. Aliases now only refine the write-own / RSVP branches, so they
can no longer over-open events (Stalwart: organizer identity does not
confer calendar write; the ACL does).
- collectUserCalendarAddresses(): merge identities + account aliases so an
alias organizer is recognised as the user's own; calendar-app fetches the
principal aliases and passes isSubscriptionCalendar through.
- event-detail-popover / event-modal: derive Edit/Delete/RSVP/read-only from
the resolver (three render states); read-only is new - no edit, no RSVP.
Tests: calendar-editability (26 cases over rights x role x subscription x
origin, incl. the alias-in-read-only regression) + calendar-participants.
tsc + eslint clean.
`npm run typecheck` failed with 8 TS2307 errors from Next's generated
route validators referencing old page paths (e.g. calendar/page.js) after
routes moved to catch-all segments. Two causes:
- The production validator (.next/types) was stale. Prepend `next typegen`
so `tsc` always runs against freshly generated route types.
- The dev validator (.next/dev/types) is a turbopack-dev-only artifact that
is absent from clean/CI checkouts and goes stale locally. `next typegen`
force-re-adds it to tsconfig `include`, so exclude it instead (exclude
overrides include), leaving route typing to the maintained .next/types.
tsc still catches real type errors; verified `npm run typecheck` is clean
and idempotent.
Both 04-all-mail and 04-shared-identity shared the same 04 prefix. Shift
shared-identity and everything after it up by one (05-shared-identity …
12-cross-view-retain), leaving 04-all-mail as the sole 04 spec. Pure file
renames; no content changes.
Adds 11-cross-view-retain.spec.ts: with the Unread cross view open, reading a
mail keeps it in the list (status + unread counter update only, no removal);
with the Starred cross view open, un-starring keeps it. Both force the
focus/push refresh that previously dropped the row and assert it survives.
Supporting test hooks:
- thread-list-item: expose data-starred on the row.
- email-context-menu: testId ctx-star / ctx-unstar on the star item.
- integration helpers: expectEmailStarred (app.ts), setFlagged (jmap.ts).
- integration policy.json: enable crossStarredViewEnabled.
Verified: 08-shared-moves-style run of 11-cross-view-retain → 2/2 passed
against the dockerized stack.
In the unread ('unread') and starred ('starred') cross views, marking a mail
read or removing its star flips the very keyword the view filters on
(notKeyword:$seen / hasKeyword:$flagged). The push-triggered refresh re-runs
that filtered query, so the mail was absent from the fresh page and the merge
dropped it - the row vanished from under the cursor on the user's own action.
Keep such rows in place: only their status + the counters update (both already
applied by markAsRead/toggleStar/batchMarkAsRead), they stay visible until the
view is re-opened. A new retainedInViewIds set records the ids the user just
read/unstarred while in these views; refreshCurrentMailbox re-inserts those rows
at their prior position via mergeRetainedRows. The set is scoped to the two
keyword cross views, cleared on navigation (fetchEmails/fetchUnifiedEmails/
fetchCrossView/exitUnifiedView), and dropped per-id when the action is reversed
(mark unread / re-star). totalEmails counts the retained rows so hasMore stays
correct.
Genuinely removed rows (deleted/moved -> absent from the local list) are not
resurrected, and a change the user did NOT make in this view (e.g. read on
another device, retain set empty) still drops as before.
Tests: stores/__tests__/email-store-retained-in-view.test.ts - read keeps the
row (unread view), unstar keeps the row (starred view), reversing/ navigation
clear retention, external read still drops, 'all' view does not retain. The
row-keeping cases fail without the fix.
refreshCurrentMailbox runs on every Email state push (delete, star, and
mark-read all raise one). It only handled real single mailboxes: in a
unified ("All Mail") or cross-account view, selectedMailbox is a virtual
id (e.g. __cross_all__ / __unified_inbox__) that no folder matches, so it
was sent to getEmails as an inMailbox filter, returned an empty page, and
the first-page merge wiped the whole aggregated list. The empty query was
previously masked by the length-based merge cutoff; af2b00dd (#592)
changed the cutoff to the page size and unmasked it.
Make refreshCurrentMailbox view-aware, mirroring loadMoreEmails /
performSearch: fan out page 0 via fetchCrossViewEmails / fetchUnifiedEmails
(search-aware) when in those views, then run the existing merge so loaded
pages are preserved. Surface cross-view fetch errors via unifiedErrors.
Test: email-store-refresh-unified asserts a push refresh in the cross-All
and unified-inbox views keeps the list and never queries the virtual id
(fails without the fix: the list wipes to []).
- track session-only dismissal per account instead of a single account id
- label the close button with a dedicated dismiss aria string in all locales
- read PWA install prompt visibility from a shared flag and subscribe in a
layout effect so an early visibility event cannot be missed
- restore real timers after the push onboarding tests
Review follow-up to #751.
The offload path could not actually run: onBeforeBlobUpload goes through
HookBus.transform, which capped every handler at the 5s default. A handler
that pushes the file to an external store blows past that, and on timeout
the error is swallowed, the original file id is kept, and the composer
attaches the big file anyway - silently - while three such timeouts in a
minute auto-disable the plugin. Buses can now carry their own per-handler
budget and this one gets 120s. The same ceiling existed inside the sandbox,
where http.post/http.fetch were bounded by the 30s default API timeout;
those two are transfers rather than round-trips and now get 120s as well.
PRIVILEGED_ONLY_METHODS listed 'upfiles.set', which matches no dispatched
method - the dispatcher calls it 'upfiles.save'. So the write was never
tier-gated despite the comment saying it was, and an untrusted plugin with
email:blob-write could replace the bytes of a file the user was about to
send. Fixed the name. Relaxing the read while that stayed open was the
wrong order.
Moving upfiles.get to email:blob-read is kept, but it is a wider grant than
the PR described: onBeforeBlobUpload hands the file id to every registered
handler, so the id is not a secret and any untrusted plugin holding
email:blob-read can read every file the user attaches. The consent copy now
says that; email:blob-write's copy was describing the read and now describes
the replace it actually gates.
Also in the composer offload path:
- sanitize the handler's html before it reaches the document and the sent
message, and insert it through the editor at the caret rather than
concatenating it after the signature and quoted block
- honour an abort that lands while the handler is offloading
- delete the right staged file when the handler re-saved it under a new id
via upfiles.save, which otherwise stranded a copy in IndexedDB forever
upfiles.get is a read, but sat behind the email:blob-write permission and
the privileged tier, which requires crypto:full plus a signed managed
bundle. That put a simple attachment-offload plugin in the same trust
bucket as crypto plugins that read stored message bytes.
The method only returns a file the user just attached in this session,
looked up by the unguessable UUID the composer itself passes to the
onBeforeBlobUpload hook. It cannot reach server-side message blobs; those
stay behind jmap.fetchBlob on the privileged tier. So gate it behind
email:blob-read (consent copy already covers attachment bytes) and drop
the tier requirement.
Note: the privileged set also lists 'upfiles.set', but the dispatched
method is named 'upfiles.save', so that entry never matched; left as-is
here, flagged in the PR.
api.http.post(path, body, options) now accepts a Blob/File body and sends
it as-is to the same-origin route instead of JSON.stringify-ing it into
'{}'. The plugin may only supply Content-Type and X-Plugin-* metadata
headers; anything else is refused, and the host attaches its credential
headers after the plugin's, so they can never be overridden. Non-Blob
bodies keep the stock JSON behaviour. The sandbox facade previously
dropped the third argument, so the options never reached the host.
A handler can now return an ExternalAttachmentResult ({externalAttachment:
true, html, text}) instead of a file id. The composer then deletes the
staged file, removes the attachment and appends the replacement content
to the body, so a plugin can push a large file to an external store and
leave a download link behind. Returning a string (or nothing) keeps the
stock upload path byte-for-byte.
Also adds plainTextMode to the upload callback deps, which the new branch
reads to pick between the html and text replacement.
Ctaoist
Linus Rath
Stefan Hildebrandt
BGandCopilot Autofix powered by AI
GitHub Copilot
Paulhenry Saux
Linus Rath
Roman Varlakov