A PatchObject key is a JSON Pointer (RFC 8620 section 5.3), so a slash
inside the member name it points at has to be escaped as `~1`. Keyword
patches were built as `keywords/${keyword}` verbatim, which is fine for
every flat keyword but wrong for a nested tag: `$label:work/clients`
became the pointer `keywords/$label:work/clients`, asking for the
`clients` member of the `$label:work` keyword rather than for the keyword
itself. The patch is not a boolean at that path, so the server rejects or
ignores it and the tag never lands.
Route the keyword patch sites - setKeyword, removeKeyword, migrateKeyword
and the category-tab batch patch - through `keywordPointer`, which escapes
per RFC 6901 the way the calendar store already does for participant ids.
Flat keywords are untouched, so `$seen`/`$flagged` and friends keep the
exact pointers they had. The dev JMAP mock reads the escaping back, or
nested tags would behave differently against the mock than against a real
server.
migrateKeyword also took any answer as success: a method-level error on
the query read as "no message carries this keyword", one on the write was
ignored outright, and a per-message refusal was invisible - so a rename
that moved nothing still reported a clean migration. It now answers with
{ migrated, refused }, because the work is batched and a failure partway
through cannot undo the batches already applied. A whole batch the server
refuses stops the rest, which it would refuse too; a single message it
refuses does not, the mail behind it being unaffected. A query error
throws, nothing having been written yet, and so does a run that migrated
nothing at all - the case where the caller must not rename a tag over
messages that all kept the old keyword.
The walk over tagged mail was not paginating safely either. It stopped at
the first page shorter than the limit it asked for, but RFC 8620 section
5.5 lets a server clamp that limit, so a clamping server left most of the
mail unmigrated. Positions are also only comparable within one queryState,
which the walk ignored: mail arriving mid-walk shifts every later page. It
now restarts when the state changes under it, and gives up - before any
write, so the caller can simply repeat - once a mailbox has moved under it
three times, rather than mixing ids from states that no longer agree. A
page carrying no id it has not already seen ends the walk, which keeps it
bounded against a server that answers every position with the same page.
Extend the existing keyword write coverage to pin the three call sites
that clear a keyword. markAsRead, batchMarkAsRead and toggleStar must
patch with null and never with false, and must still send true when
setting the keyword.
The three removal cases fail against the previous boolean form, one per
call site. The three set cases pass either way, since that path was
already correct.
Review follow-ups on the unclosed-angle split:
- The paste path fell back to splitMailbox for every entry, which keeps the
last angle run only and folds everything ahead of it into a display name.
`Ann <a@x.com Bob <b@y.com` became one chip and a@x.com disappeared without
even landing in `invalid`. Take that path only when the prefix is not an
address itself.
- The new separator branch ignored `inGroup`, so an unclosed `<` inside RFC
5322 group syntax shattered the group its sibling branch keeps intact.
- angleRunCloses now scans quote-aware, so a `>` inside a quoted display name
no longer counts as closing the run, and it starts past `from` so the helper
is correct whether it gets the separator or the opening `<`.
The suite intermittently failed "settingsSync ... SESSION_SECRET_FILE" under
full parallel load (passed in isolation). Two causes:
- env vars were mutated directly on the shared process.env, so other test
files in the same worker could leak in. Use vi.stubEnv + unstubAllEnvs so
each test's env is isolated and auto-restored, and stub the managed vars to
undefined in beforeEach for a clean baseline.
- both SESSION_SECRET_FILE tests wrote a shared relative ./session-secret and
removed it with a fire-and-forget async unlink; under load the pending
unlink from the first test fired during the second (after write, before
read), so the file was missing. Use a unique temp file per test, removed
synchronously in afterEach.
Verified: 17/17, stable across repeated runs and with hostile env
(APP_NAME/SETTINGS_SYNC_ENABLED/SESSION_SECRET) set in the real environment.
The "This event only" edit path built nested JMAP patch pointers
(recurrenceOverrides/<recurrenceId>/<field>). On the first edit of an
occurrence the override object doesn't exist yet, and JMAP pointers can't
create a missing intermediate object, so Stalwart returned notUpdated and
the edit failed with two error toasts.
Build the override as a single pointer recurrenceOverrides/<recurrenceId>
whose value is the occurrence's field set (dropping identity/series keys),
mirroring the working "delete this occurrence" path. Extracted as the pure,
tested helper buildRecurrenceOverridePatch.
Closes#774.
A calendar holding only tasks (VTODO, e.g. from tasks.org) appeared in the
event calendar UI as an empty calendar. Stalwart's Calendar/get exposes no
supported-component set, so tasks-only-ness is derived from the calendar's
objects (mirroring the existing event/task split): a calendar is tasks-only
iff it holds >=1 object and every object is a task.
- getTasksOnlyCalendarIds scans an account's objects (CalendarEvent/query +
minimal /get), early-exits once every calendar has an event, only marks
after a full scan, and fails open on the cap or any error.
- getAllCalendars/getCalendars annotate calendars with isTasksOnly.
- Such calendars stay in the store for the tasks view (task picker, task-list
grouping) but are filtered from the event sidebar and default selection.
Verified against Stalwart v0.16.16: the tasks-only calendar is flagged, the
default (with an event) is not.
Closes#761.
The self-sent handling from #703 (reply to your own thread message ->
address the original recipients, ignore Reply-To) was entered whenever
From matched one of the user's identities. Contact-form / helpdesk mail
commonly arrives From the user's own address with the real correspondent
in Reply-To, so that branch ignored the Reply-To and looped the reply
back to the user's own To.
Only take the self-sent branch when there is no EXTERNAL Reply-To: a
Reply-To resolving to a non-own address is an explicit RFC 5322 directive
and now wins even for self-sent messages (#776). A Reply-To that is only
our own address stays ignored for self-sent (#703 preserved).
Closes#776.
connectSSE() overwrote sseAbortController with a new controller without
aborting the previous one, so re-running setupPushNotifications (which
happens per connected account and on fairly frequent effect changes)
orphaned SSE streams. readSSEStream only unwinds on abort/done, so each
orphan held its HTTP/1.1 socket forever; with multiple accounts this
starved normal JMAP POSTs, which queued and timed out (~30s).
- connectSSE: abort any prior stream (+ its ping monitor / pending
reconnect) before opening a new one -> exactly one live SSE per client.
- startPollingFallback: bail if a polling interval already exists.
- checkForStateChanges: skip while a poll is already in flight
(stateCheckInFlight) so overlapping triggers don't stack POSTs.
Closes#781.
markAsRead, batchMarkAsRead and toggleStar cleared a keyword by patching
it to false, sending keywords/$seen: false or keywords/$flagged: false.
RFC 8620 section 5.3 removes a property from a PatchObject by setting it
to null, and RFC 8621 section 4.1.1 defines the keywords map as holding
true for every key, so false is not a valid way to clear one.
A server that enforces this rejects the update with invalidProperties.
The client does not inspect notUpdated, so marking a message unread or
removing a star looked like it had worked, the sidebar counts moved, and
the next refresh restored the old state from the server. Stalwart accepts
both forms, which is why this went unnoticed.
Send null instead, matching removeKeyword and the integration test
helpers, which already patch keywords this way.
The Files store was the only account-scoped store never wired into the
account lifecycle: account-state-manager clears email/contacts/calendar/
filter/identity/vacation on a switch, but not files. Because the Files
store is a global that outlives the FilesApp component, the previous
account's client + resources lingered in memory and resurfaced whenever
the drive was reopened after a switch - most visibly when the switch
happened on the mail view (FilesApp unmounted), the common normal-shell
flow that the FilesApp re-init effect alone does not cover.
Clear the Files store centrally in clearAllStores() alongside the other
account-scoped stores, so the reset happens on every switch path.
Tests:
- lib/__tests__/account-state-manager.test.ts: assert clearAllStores()
resets the Files drive (client, resources, pathStack, selection,
supportsFiles) with no cross-account leak.
- integration/tests/13-files-account-switch.spec.ts (dockerized Stalwart,
JMAP FileNode): seeds a distinct top-level folder in alice's and bob's
drives and asserts the on-screen drive swaps on a switch with no stale
folder - both switching off the Files view (mail -> switch -> open
Files) and on it (rail account switcher). Verified: passes with the fix,
fails (path 1) without it. New FileNode seeding helpers in helpers/jmap.ts;
openFiles/openMailView/driveEntry nav helpers in helpers/app.ts.
Linus Rath
Linus Rath
KazNIISA IT
zisisvakras
Stefan Hildebrandt
Paulhenry Saux