Skip to content

fix: JMAP fetching and moving#1446

Merged
andrinoff merged 2 commits into
floatpane:masterfrom
masukomi:jmap
Jun 6, 2026
Merged

fix: JMAP fetching and moving#1446
andrinoff merged 2 commits into
floatpane:masterfrom
masukomi:jmap

Conversation

@masukomi
Copy link
Copy Markdown
Contributor

@masukomi masukomi commented Jun 5, 2026

What?

in jmap.go

  • added a resolveUID function
  • added a resolveUIDByQuery function

Why?

Email body fetch always failed with "jmap: no cached ID" (backend/jmap/jmap.go)
FetchEmailBody relied on p.idToJMAPID being pre-populated by FetchEmails on the same Provider instance. However, the fetcher layer creates a fresh Provider for every call, so the map is always empty when FetchEmailBody runs.

The fix adds a resolveUID method that checks the in-memory cache first (fast path when the daemon reuses the same instance), then falls back to resolveUIDByQuery. The fallback issues an Email/query for the folder, reads the JMAP string IDs directly from QueryResponse.IDs, hashes each one with FNV-32a, and returns the match — also warming the cache as a side effect.

Email body fetch always failed with "jmap: no cached ID" (backend/jmap/jmap.go)

FetchEmailBody relied on p.idToJMAPID being pre-populated by FetchEmails on the same Provider instance. However, the fetcher layer creates a fresh Provider for every call, so the map is always empty when FetchEmailBody runs.

The fix adds a resolveUID method that checks the in-memory cache first (fast path when the daemon reuses the same instance), then falls back to resolveUIDByQuery. The fallback issues an Email/query for the folder, reads the JMAP string IDs directly from QueryResponse.IDs, hashes each one with FNV-32a, and returns the match — also warming the cache as a side effect.
@masukomi masukomi requested a review from a team as a code owner June 5, 2026 15:02
Copy link
Copy Markdown
Member

@floatpanebot floatpanebot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @masukomi! Please fix the following issues with your PR:

  • Title: Is too long (48 characters). The PR title must be strictly under 40 characters.

@floatpanebot floatpanebot added bug Something isn't working size/M Diff: 51–200 lines labels Jun 5, 2026
@masukomi masukomi changed the title fix(jmap): fetching JMAP email body always fails fix(jmap): fetching JMAP email body fails Jun 5, 2026
Copy link
Copy Markdown
Member

@floatpanebot floatpanebot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @masukomi! Please fix the following issues with your PR:

  • Title: Is too long (41 characters). The PR title must be strictly under 40 characters.

@andrinoff andrinoff changed the title fix(jmap): fetching JMAP email body fails fix: fetching JMAP email body fails Jun 5, 2026
@floatpanebot floatpanebot dismissed stale reviews from themself June 5, 2026 15:08

Formatting issues have been resolved. Thank you!

@floatpanebot floatpanebot added the backport/v1 Merge to master and backport to release/v1 label Jun 5, 2026
PROBLEM

The JMAP backend maps server-assigned string IDs to uint32 UIDs via an in-memory cache (idToJMAPID). Operations like archive, delete, move, and mark read/unread all called lookupJMAPID to translate a UID back to a JMAP ID before making API calls.

This cache is only populated when FetchEmails is called on the same Provider instance. The daemon creates one long-lived Provider per account for email mutations, but FetchEmails runs through a separate fetcher layer that creates fresh Provider instances per call. As a result, the mutation provider's cache was always empty, and every archive/delete/move/mark-read attempt failed with jmap: no cached ID for UID <n>.

FIX

Changed MarkAsRead, MarkAsUnread, DeleteEmail, ArchiveEmail, and MoveEmail in backend/jmap/jmap.go to call resolveUID instead of lookupJMAPID.  resolveUID checks the in-memory cache first (fast path, no network), then falls back to resolveUIDByQuery which issues an Email/query for the folder to fetch all IDs, hashes each one to find the match, and warms the cache as a side effect. Subsequent operations on the same provider instance hit the cache.
@floatpanebot floatpanebot added the area/daemon Daemon / RPC label Jun 6, 2026
@masukomi
Copy link
Copy Markdown
Contributor Author

masukomi commented Jun 6, 2026

added the commit about moving things. Will check in on the CI results later.

@andrinoff andrinoff changed the title fix: fetching JMAP email body fails fix: JMAP fetching and moving Jun 6, 2026
@andrinoff
Copy link
Copy Markdown
Member

andrinoff commented Jun 6, 2026

there are some issues that will need further follow ups. but for now, this is lgtm.

To not forget, follow ups:

  • Configurable limit of UID (currently set at hardcoded 10k)
  • Potentially expensive fallback runs on the hot path for every per-UID action. (resolveUIDByQuery performs a full Email/query over the entire folder (up to 10k IDs) and hashes every single one whenever the cache misses.)

@andrinoff andrinoff added this pull request to the merge queue Jun 6, 2026
Merged via the queue into floatpane:master with commit 496a775 Jun 6, 2026
34 checks passed
floatpanebot pushed a commit that referenced this pull request Jun 6, 2026
## What?
in `jmap.go`

- added a `resolveUID` function
- added a `resolveUIDByQuery` function

## Why?
Email body fetch always failed with "jmap: no cached ID"
(backend/jmap/jmap.go)
FetchEmailBody relied on p.idToJMAPID being pre-populated by FetchEmails
on the same Provider instance. However, the fetcher layer creates a
fresh Provider for every call, so the map is always empty when
FetchEmailBody runs.

The fix adds a resolveUID method that checks the in-memory cache first
(fast path when the daemon reuses the same instance), then falls back to
resolveUIDByQuery. The fallback issues an Email/query for the folder,
reads the JMAP string IDs directly from QueryResponse.IDs, hashes each
one with FNV-32a, and returns the match — also warming the cache as a
side effect.

(cherry picked from commit 496a775)
@floatpanebot floatpanebot added the backported Cherry-picked onto release/v1 label Jun 6, 2026
@floatpanebot
Copy link
Copy Markdown
Member

Cherry-picked 496a775 onto release/v1.

@floatpane floatpane deleted a comment from andrinoff Jun 6, 2026
@floatpane floatpane deleted a comment from andrinoff Jun 6, 2026
@floatpane floatpane deleted a comment from masukomi Jun 6, 2026
@floatpane floatpane deleted a comment from andrinoff Jun 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/daemon Daemon / RPC backport/v1 Merge to master and backport to release/v1 backported Cherry-picked onto release/v1 bug Something isn't working size/M Diff: 51–200 lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants