Two real bugs in the previous WS-push commit, both found while building the
integration test for it (not theoretical - each reproduced and verified
before and after the fix):
1. proxy.ts's production CSP (`connect-src 'self' https:`) has no `wss:`
term, so `new WebSocket(...)` was blocked before any network attempt at
all - confirmed by listening for `securitypolicyviolation` against the
real reference server (stalwart.sandbox.vnc.de, HTTPS): the WS feature
was entirely inert in a production build, for every server, not just
ones with an incompatible auth model. Fixed by adding `wss:` alongside
`https:` in production - no new trust surface, since `https:` here
already allows fetch/XHR to any TLS host (needed for
ALLOW_CUSTOM_JMAP_ENDPOINT / multi-server setups), so extending that same
model to WebSocket is consistent, not a new precedent. Verified after the
fix: the same probe now reaches the network and gets a real (expected)
auth rejection from Stalwart instead of a CSP block.
2. lib/jmap/client.ts's circuit breaker (5 attempts, 1s/30s backoff) could
take up to ~31s to give up on WS and fall back to SSE. Against a server
that fails the handshake instantly and deterministically every time (the
auth-header limitation documented in the previous commit), that's ~31s
of NO live push at all - WS hasn't succeeded and hasn't given up yet, so
SSE never starts connecting, and any mail delivered in that window was
silently missed (SSE only streams changes from the moment it connects,
no catch-up). Reproduced directly: a real SMTP delivery sent during that
window never reached the notification bridge.
Fixed two ways:
- Tightened the ladder to a 200ms base / 5s cap / 3-attempt circuit
breaker (worst case ~1.75s instead of ~31s) - still genuine
exponential-with-jitter backoff, just tuned for a failure mode that's
fast and deterministic rather than slow and flaky. A slow/real
network issue is unaffected: a hanging attempt is still bounded by
the browser's own WebSocket connect timeout, not by these constants.
- setupPushNotifications() now primes a polling baseline
(fetchCurrentStates()) in parallel with the WS attempt, and
fallbackFromWebSocket() diffs against it (checkForStateChanges())
BEFORE connectSSE()/startPollingFallback() get a chance to erase that
opportunity. This is what actually closes the gap rather than just
shrinking it: it catches a change that happened to the primary
account during the (now much shorter) WS retry window.
electron/main.ts also gets a test-only escape hatch (ELECTRON_LOAD_URL): set
it to skip spawning the standalone server and load that URL instead. Real
users and every packaging/CI path never set it - added because verifying
the fixes above against this repo's own local Stalwart fixture (deliberately
plaintext HTTP - integration/webmail.Dockerfile makes the identical
trade-off for the browser-based suite) needs a dev-mode Next.js server
(proxy.ts only widens connect-src for plain http/ws in dev), not the
production standalone build electron/main.ts normally boots.
next.config.ts: added 127.0.0.1 to allowedDevOrigins alongside the existing
LAN entry - electron/main.ts always loads its window at 127.0.0.1, so a
dev-mode Electron run (only used by the escape hatch above) needs it in this
allowlist the same as any other cross-origin dev client would.
Verified: full lib/__tests__ JMAP suite still green (158/158); npm run
test:electron still green (4/4); the raw WebSocket probe against the real
sandbox now reaches the network post-fix instead of being CSP-blocked.
Phase 1 step 6 of VNCprodbuild, resolving the step-5 DECISION gate (human
confirmed: WebSocket push, not polling, not quit-to-tray).
lib/jmap/client.ts: getWebSocketUrl() discovers the push endpoint from the
session's own urn:ietf:params:jmap:websocket capability (mirrors
getEventSourceUrl()'s existing pattern) - not hardcoded to any one server,
rewritten to the client's own host the same way apiUrl/downloadUrl/
eventSourceUrl already are (rewriteWebSocketUrl(), scheme-aware since ws/wss
can never share an origin string with the client's http/https serverUrl).
setupPushNotifications() now tries WS first when advertised, falling back to
the existing SSE/polling chain when not. connectWebSocket() subscribes via
WebSocketPushEnable and routes incoming StateChange frames through the exact
same stateChangeCallback that SSE/polling already feed - so
stores/email-store.ts's handleStateChange (mailbox/email refresh, scheduled
mail, calendar, filters) and handleNewEmailNotification (the new-mail toast/
sound signal) all work unchanged regardless of which transport delivered the
change.
Reconnect/backoff: exponential with full jitter (1s base, 30s cap - unlike
SSE's fixed 3s retry, explicitly requested since a long-lived WebSocket can
be dropped by sleep/network-switch/idle-proxy repeatedly in a row). An
app-level heartbeat (Core/echo every 30s, force-reconnect after 90s of
silence) catches connections that report readyState OPEN long after the
underlying path is actually gone, mirroring the existing SSE ping monitor.
Circuit breaker (wsConsecutiveFailures/wsPermanentlyDisabled): gives up on
WS after 5 CONSECUTIVE handshake failures (never reaching "open" - a
connection that opened fine and dropped later doesn't count) and falls back
to SSE/polling for the rest of the client instance's life. This is not
theoretical - verified empirically against the actual sandbox server this
was built against:
curl -i -H "Connection: Upgrade" -H "Upgrade: websocket" \
-H "Sec-WebSocket-Version: 13" -H "Sec-WebSocket-Key: ..." \
-H "Sec-WebSocket-Protocol: jmap" https://stalwart.sandbox.vnc.de/jmap/ws
-> 401 Unauthorized, WWW-Authenticate: Bearer/Basic
Stalwart's /jmap/ws requires the same HTTP Authorization header as every
other JMAP endpoint on the upgrade request itself, and the browser
WebSocket constructor cannot attach custom headers to that handshake (a
WHATWG spec restriction - credentials-in-URL is also explicitly rejected).
Every connection attempt from this renderer-side client will therefore fail
against Stalwart specifically and fall back to SSE (which keeps working
exactly as before - zero regression). Implemented for real anyway, not
stubbed: it's fully spec-correct and activates automatically against any
server whose WS endpoint doesn't share this auth model (e.g. behind a
cookie-authenticating proxy), and the alternative (opening it from
Electron's main process via a header-capable client, which would need raw
credentials piped over IPC from the renderer) is a materially bigger
security-sensitive change than what was scoped here. Documented in detail
in the code comments above the new fields.
lib/jmap/client-interface.ts + lib/demo/demo-client.ts: getWebSocketUrl()
added to the interface (demo client returns null, matching
getEventSourceUrl's existing stub).
app/(main)/[locale]/page.tsx: the existing "new mail arrived" effect (which
already plays a sound, transport-agnostically, whenever
stores/email-store.ts sets newEmailNotification for a genuine new top-of-
inbox message) now also calls lib/electron-bridge.ts's
showElectronNotification() when isElectronShell() - firing the native
notification bridge built in the step-3 commit, gated on the same
emailNotificationsEnabled setting the sound already uses. Fallback title/
body text ("New mail" / "(no subject)") matches public/sw.js's existing
push-notification fallback strings rather than introducing new i18n keys
for a rarely-hit edge case.
Verified: full lib/__tests__ JMAP suite green (158/158 across 13 files,
excluding one pre-existing unrelated flaky test - jmap-client-resilience's
ping-failure-reconnect-ordering assertion uses real timers and fails
~75% of the time on both this branch's base commit and this change,
confirmed by running the untouched baseline the same way). npm run
test:electron still green (4/4) after a full rebuild.
Phase 1 step 3 of VNCprodbuild. electron/preload.ts's contextBridge now
exposes window.vnc.showNotification(title, options), routed via
ipcRenderer.invoke("vnc:show-notification") to a new ipcMain.handle in
electron/main.ts that calls Electron's own Notification API. This is the
desktop shell's native notification path - it sits alongside, not in place
of, the browser/PWA's service-worker push path (public/sw.js's push/
notificationclick handlers + lib/web-push.ts), which is untouched.
lib/electron-bridge.ts gives the renderer a `isElectronShell()` +
`showElectronNotification()` wrapper so app code can detect the shell and
use the native path instead of/alongside SW push - not wired to any real
mail-delivery trigger yet, that's Phase 1 steps 4-6 (JMAP realtime
capability investigation, the background/foreground strategy decision, and
implementing it).
Extended e2e/electron-smoke.spec.ts to prove the IPC plumbing actually
fires end-to-end: calls window.vnc.showNotification from the renderer and
asserts the round-trip resolves (not that a real OS toast appears - not
observable in CI). Verified locally: the call resolves {"shown":true} on
this machine, confirming it genuinely reaches Electron's Notification API
and back, not just that window.vnc exists.
Also fixes a real bug caught by this step's typecheck: the smoke test's
Playwright Page variable was named `window`, shadowing the DOM global
inside every evaluate() callback and silently breaking their types. Renamed
to `appWindow`.
All 4 smoke-test assertions green: npm run build:electron && npm run
test:electron.
`info.hooks` is self-reported by the sandboxed bundle, and the loader
registered any recognised hook name without checking permissions. An
untrusted, null-origin plugin could therefore claim `onRenderEmailBody`
and replace the rendered body of any opened email without ever holding
`email:render-takeover` — the permission was enforced only by the
one-time consent dialog, i.e. it gated what the user was *asked*, not
what the host *allowed*.
Add HOOK_PERMISSIONS covering the hooks that can read message content,
alter outgoing mail, or observe key state: render takeover, the three
send-interception hooks, bulk-content hooks, attachment upload, and the
four S/MIME hooks. Hooks absent from the map stay unrestricted (UI
observation, toasts, navigation), so ordinary plugins are unaffected.
Refused hooks fail closed and log the missing permission by name — a
silently inert hook is far harder to diagnose than a refused one.
Export hasPermission() from host-api rather than reimplementing the rule
in the loader, so the hook gate and the RPC gate cannot drift apart.
Remaining ~200 hooks are tracked as B-09.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add optional logoLightUrl/logoDarkUrl to InstalledTheme + resolveThemeLogo()
helper; set logos on vnclagoon + src themes; login page and nav-rail prefer the
active theme's logo, falling back to the global config logo. So switching theme
switches the whole brand. 0 type errors.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Extend the vnclagoon skin: solid navy login card, cyan hairline border, a thin
cyan accent strip on top, soft cyan glow (dark), and an ambient cyan wash behind
the card on the login page. Scoped to the login card's unique class combo.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add builtin-vnclagoon theme (cyan #00D4FF accent on navy #0A0E1A, DM Sans body
+ Syne headings, self-hosted OFL fonts); set as default theme policy; default
mode dark. Add VNCmail wordmark SVGs (on-dark/on-light) + wire logo/company via
k8s secret template. Placeholder wordmark — swap official styleguide SVG.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
If you have a large number of tags, getTagCounts would not be able to get the
unread counts because it did not respect maxCallsInRequest, even though the
value was actually read out, it was just ignored. There are also other places
where the limits were not respected.
Batching is now generalized in a helper that also takes maxObjectsInSet, which
also was ignored, into account and is applied to all functions.
In addition, the dev mock now also advertises and enforces the limits, so these
issues can get picked up during development.
Possible closes#699
Possibly closes#399
Previously tags where very much focused on color coding email and less about
adding additional information. They were also visualized in different ways in
different locations.
This commit gets rid of all "Color-coding" references, aligns visualization of
the tags across the whole project and tries to improve user experience of using
tags in general.
A search box is shown in the tagging control so the user can quickly search for
a tag if they have a huge (more than 10) amount of tags.
- levels are joined by forward slashes in the keywords
- behaviour is opt-in for now
- long paths are shortened if there is not enough display room
Closes#687.
process.env coerces assigned values to strings, so
process.env.TZ = originalTZ left the literal string "undefined"
behind (instead of clearing TZ) when it was unset before the test
ran. Delete the var in that case instead of assigning undefined.
Setting process.env.TZ at module scope without restoring it could leak
into other test files sharing the same Vitest worker. Match the
beforeAll/afterAll restore pattern already used in
lib/__tests__/calendar-utils.test.ts.
buildForwardAsAttachmentPayload named the synthetic .eml attachment
using the user's own emailDownloadTemplate, which by default embeds
sender/recipient display names. Since this attachment can go to an
external recipient (e.g. an upstream spam gateway, or anyone else),
the filename now always renders as "{date}-{subject}.eml" regardless
of the user's configured template, while still honoring their
space/case/diacritics preferences.
buildForwardAsAttachmentPayload called buildForwardSubject(email.subject,
forwardPrefix) unconditionally, and buildForwardSubject("", prefix)
returns just the bare prefix rather than "". Normal Forward doesn't do
this - EmailComposer's getInitialSubject() returns "" outright when
!replyTo?.subject, only calling buildForwardSubject when there's an
actual subject to prefix. So forwarding a subject-less message as an
attachment produced "Fwd:" as the subject, while normal Forward left
it blank.
Only call buildForwardSubject when email.subject is truthy, matching
getInitialSubject()'s behavior exactly. Add a test.
Caught by GitHub Copilot's automated PR review.
buildForwardAsAttachmentPayload called emailExportFilename(email) with
no options, always using the default naming template regardless of
the user's configured emailDownloadTemplate (and space/case/diacritics
transforms) - the same settings the neighboring "Export as .eml"
action already respects. That could produce inconsistent .eml
filenames between the two actions for the same message.
Accept an optional EmailFilenameOptions parameter and pass it through.
handleForwardAsAttachment now reads the same settings
email-viewer.tsx's emailFilenameOptions useMemo does, via
useSettingsStore.getState() (a one-off read inside an event handler,
matching this file's existing pattern, rather than a new reactive
subscription). Add a unit test covering a custom template.
Caught by GitHub Copilot's automated PR review.
Adds a "Forward as attachment" action to the message overflow menu
(desktop and mobile), right beside the existing "Export as .eml"
action. Opens a new forward-mode compose window with the original
message attached as a message/rfc822 file instead of quoted inline -
useful for reporting spam/phishing to an upstream gateway that expects
the raw original as an attachment (the primary motivating use case:
gateways like MxGuarddog require complete original headers, including
the full mail path, for scanning), or for preserving a message's exact
formatting/headers when forwarding.
Implementation reuses the composer's existing attachment-carry-forward
mechanism (the `attachments` useState initializer in
email-composer.tsx already carries a forwarded message's own
attachments into the new compose via `replyTo.attachments`) - this
just adds one synthetic entry representing the whole original message,
referenced by its existing blobId. No re-fetch or re-upload needed,
since JMAP blobs are account-scoped rather than per-email. The inline
quote-header step (prepareComposerQuoteHeader) is skipped, so the body
starts blank instead of quoting the original.
The core "build subject + attachment entry" logic is extracted into a
pure, unit-tested helper (lib/forward-as-attachment.ts) rather than
left inline in the already-large page component.
Adds the forward_as_attachment locale key to all 24 locales (English
text as a placeholder pending translation, following the existing
add-a-key convention) to satisfy the translations completeness test.
The explicit Email/set destroy workaround for the duplicate-on-move bug is
removed now that the root cause is filed upstream (support.stalw.art #1150:
onSuccessDestroyOriginal destroys the copy's create-id instead of the source
id). copyEmailAcrossAccounts keeps requesting onSuccessDestroyOriginal, so the
move self-heals once Stalwart ships the fix.
Kept: the keyword-preservation fix (carry the source keywords into Email/copy)
so the moved message keeps its read state.
Tests: 08-shared-moves still asserts delivery + read-state on every
cross-account case; the source-removal checks are re-pinned test.fail, scoped
to a nested describe, until #1150 is fixed. Suite green (5 pass, 3 expected-fail).
Moving a message across the account boundary (own ↔ shared folder, or
between two owners' shared folders) left the original in the source
folder and showed the moved copy as unread. Same-account moves were fine.
Cause (verified against a live Stalwart):
- Email/copy drops keywords unless the create sets them, so the copy lost
$seen and arrived unread.
- onSuccessDestroyOriginal is unreliable — the implicit destroy reports
notFound and leaves the original behind (flaky), so the move duplicated.
Fix (copyEmailAcrossAccounts): read the source keywords and carry them
into the Email/copy create, then destroy the original with an explicit
Email/set on the source account instead of onSuccessDestroyOriginal.
Tests: 08-shared-moves now asserts the source is gone and the read state
survives on every cross-account case, and adds a cross-owner shared →
shared move (alice's folder → bob's folder). Confirmed red on the old
code (3 cross-account cases fail), green with the fix.
Moving a message to a folder in another account (own ↔ delegated/shared) via the
"Move to" context menu was a no-op — the handlers always issued a single-account
Email/set, which can't move between JMAP accounts. Drag-and-drop already routed
these correctly; the context menu never did.
Add moveToMailboxCrossAware: it detects a cross-account destination (own and
shared mailboxes both carry accountId) and routes through the drag-and-drop
crossAccountMoveEmails pipeline, else falls back to the single-account move.
Fix the pipeline for delegated folders too: a client can't stage a blob in a
delegated account (Blob/upload → blobNotFound), so importing into a shared folder
failed. When one client reaches both accounts, use a server-side JMAP Email/copy
(+ destroy original) instead of blob copy+import; the blob path is kept only for
separate cross-server login accounts. Adds client.copyEmailAcrossAccounts.
Unit tests for the dispatch; the two 08-shared-moves specs are un-pinned. Full
docker integration suite green (37 passed).
Reopening a draft reset the composer's From to the default identity. The
edit-draft handler matched the draft's saved From against the active-account
identity list by email only, so two identities sharing an address (a default +
an alias differing by name) collided — the wrong one was picked, or with
cross-account namespaced ids none was.
Add findDraftIdentityId (name+email, normalized, +tag fallback) and match against
the same list the composer renders (the flat cross-account list when multi-
account is on). Wired into both the classic and Pro edit-draft paths. Unit tests
plus the un-pinned 07-drafts integration spec.
The calendar fan-out probes every shared/group account on suspicion,
because Stalwart does not always advertise calendar capability on group
accounts. A shared account that grants no calendar access at all rejects
that probe - and did so again on every calendar interaction: each range
change re-queried the account and logged a red console error
("You do not have access to account X") while working fine otherwise.
Remember the rejection instead: the thrown query error now carries the
JMAP error type, an access rejection for a probed secondary account is
logged once at debug level, and both fan-out loops (events and calendar
lists) skip the account for the rest of the session. Genuine failures on
the primary account keep the error log. Two regression tests cover the
probe-once behavior and the calendar-list skip.
Since the per-account push setup (#281), every account switch tears down
and re-creates push notifications for all connected clients. Aborting an
SSE connect that is still in flight lands in the fetch rejection handler,
which treated it as a network failure:
- fallbackToPolling() started an unsupervised 3s state poll on a client
whose push had just been intentionally closed, after the cleanup that
would have removed it had already run.
- The late rejection also nulled sseAbortController, orphaning the
replacement connection set up right after: it could never be aborted
again and reconnected itself in parallel once the server closed it.
Rapid switching multiplied both effects until the server's concurrency
limit stalled the app entirely.
Each connect attempt now tracks its own AbortController: an aborted
attempt returns silently instead of falling back to polling, and the
end-of-stream reconnect only fires if the stream is still the current
one. Regression tests simulate the switch churn both ways.
Fixes#592.
refreshCurrentMailbox merges the refreshed first page with the already
loaded list, appending existing entries beyond a cutoff. That cutoff
was derived from the refreshed list's length - so whenever a folder
shrank, the fresh page was shorter than the stale list and the loop
re-appended the deleted rows from stale local state, despite the
comment right above promising the opposite.
The visible result is the reported bug: after sending a draft, the
Drafts view keeps showing a ghost row for the already-destroyed draft.
The send actually succeeded - resending the ghost delivers the mail
again, which we reproduced with a live JMAP trace: four successful
submissions, an empty server-side Drafts folder, a notFound ghost id,
and five delivered copies. Deriving the cutoff from the page size
fixes the shrink case while preserving the merge's intent for
arrivals and loaded deeper pages; regression tests cover all three
shapes.
Also surface post-send filing failures instead of dropping them, as
flagged in 4dc76bbb's follow-up note: a rejected onSuccessUpdateEmail
patch or old-draft destroy now logs the server's error details and
returns a filingError on SendEmailResult, and the UI shows a warning
toast (all 23 locales) so a stale draft row is never again mistaken
for a failed send. A plugin veto of the send leaves a debug trace.
Bernd Rodler