[bengotow]

Fixes MAILSPRING-CLIENT-AH — 17 users impacted, 1084+ occurrences over the last two months, "core:delete-item" (move-to-trash) throwing and doing nothing.

What I observed

The Sentry error is:

Error: ChangeFolderTask: You must provide a single folder. Got object: {"id":"...","role":"trash","path":"Trash","__cls":"Label"}

thrown from the ChangeFolderTask constructor (change-folder-task.ts:74-78), which asserts this.folder instanceof Folder. The stack trace shows it originates from the core:delete-item keyboard/toolbar command → TaskFactory.tasksForMovingToTrash → new ChangeFolderTask({ folder: trash, ... }), where trash is a Label, not a Folder.

CategoryStore.getTrashCategory()/getSpamCategory() return whatever category object the sync engine created for the trash/spam role — a Folder for IMAP-style accounts, but a Label for Gmail-style (label-based) accounts. TaskFactory.tasksForMovingToTrash and tasksForMarkingAsSpam always wrapped the result in a ChangeFolderTask, so any account where trash/spam is a Label reliably crashed on delete/spam actions.

This is exactly the same class of bug that tasksForArchiving and tasksForMarkingNotSpam already guard against — both branch on instanceof Label and use ChangeLabelsTask instead — and the same pattern used in thread-snooze's moveThreads. tasksForMovingToTrash/tasksForMarkingAsSpam were just missing that check. ChangeLabelsTask's own description() even already has dedicated "Trashed"/"Marked as Spam" copy for label adds, so the label-based path was clearly intended, just not wired up here.

The fix

In app/src/flux/tasks/task-factory.ts, tasksForMovingToTrash and tasksForMarkingAsSpam now check whether the category is a Label. If so, they build a ChangeLabelsTask that adds the trash/spam label and removes the inbox label (mirroring tasksForArchiving's inbox-removal and thread-snooze's add/remove pattern), instead of constructing an invalid ChangeFolderTask. Non-label accounts keep using ChangeFolderTask as before.

Also corrected the stale doc comments on CategoryStore.getTrashCategory/getSpamCategory, which claimed to always return a Folder.

Testing

• Added unit tests in app/spec/tasks/task-factory-spec.ts covering both the Folder and Label category cases for tasksForMovingToTrash and tasksForMarkingAsSpam.
• npm run typecheck and npm run lint pass on the changed files (pre-existing, unrelated errors from missing optional native/test dependencies in this sandbox were not introduced by this change).
• Wasn't able to run the full Electron-hosted Jasmine suite (npm test) in this headless sandbox — it hung with no output under Xvfb — so please double check CI runs clean.

---

Generated by Claude Code

[indent-staging]

Important

Indent Zero has shut down and no longer reviews pull requests.
To get this pull request reviewed by Indent instead:

1. Sign up for Indent
2. Install Indent on your repositories
3. Turn on code review
4. Comment @indent on this pull request

Step 4 is only needed for pull requests that were already open when you switched. After that, Indent reviews new pull requests on its own.

To stop this notice, turn PR reviews off in Indent Zero.

[indent]

PR Summary

Fixes a high-frequency crash (Sentry MAILSPRING-CLIENT-AH: 17 users, 1084+ occurrences) where "Move to Trash" and "Mark as Spam" threw ChangeFolderTask: You must provide a single folder and did nothing on Gmail-style (label-based) accounts. TaskFactory unconditionally wrapped the trash/spam category in a ChangeFolderTask, but on Gmail those categories are Labels, not Folders. The fix branches on instanceof Label and emits a ChangeLabelsTask instead, matching the pattern already used by tasksForArchiving, tasksForMarkingNotSpam, and thread-snooze's moveThreads.

• tasksForMovingToTrash / tasksForMarkingAsSpam: when the category is a Label, build a ChangeLabelsTask (add trash/spam, remove inbox when inbox is a Label); Folder accounts keep the existing ChangeFolderTask path.
• CategoryStore.getTrashCategory / getSpamCategory: doc comments updated to note they may return a Label on Gmail-style accounts.
• Added spec coverage for both the Folder and Label branches of both methods.

Issues

No issues found.

CI Checks

The test check failed during "Install dependencies", not in the tests themselves — the better-sqlite3 native module (v12.11.1) fails to compile against the CI runner's Node 20 / V8 headers, so the suite never ran. This is a toolchain/environment issue unrelated to this PR, which only touches TypeScript (task-factory, category-store, and specs).

test (github-actions)

• Failed in the npm install postinstall step, before any test ran: node-gyp rebuild of better-sqlite3@12.11.1 errors out compiling against the runner's V8 headers (v8-primitive.h:658: error: expected primary-expression before 'public', make ... Error 1). This is a native-module/toolchain incompatibility (better-sqlite3 vs the installed node-gyp v12.2.0 / Node v20.20.2 headers), independent of this PR, which changes only TypeScript files. Re-running won't help until the dependency/toolchain is bumped (e.g. update better-sqlite3 or node-gyp).

[bengotow]

The test check is failing, but it's not this PR's failure: better-sqlite3's native build fails to compile against Electron 43.4.1's V8 headers on the CI runner's GCC version (error: expected primary-expression before 'public' in v8-primitive.h, from a V8_EXPORT/__attribute__ macro incompatibility). This diff only touches app/src/flux/tasks/task-factory.ts, app/src/flux/stores/category-store.ts, and app/spec/tasks/task-factory-spec.ts — no dependency or build config changes.

I confirmed this is pre-existing on master: the same test workflow has failed with this exact error on every commit since #2818 ("Update electron from 41.7.2 -> 43.4.1") landed, 12+ consecutive runs including the current tip (b22339b).

No merged fix exists yet to port in. There's an open candidate, #2825 ("Update to electron 44"), but it hasn't had CI run on it at all yet and carries a large, unrelated lockfile-format migration (npm lockfile v2→v3) plus removal of custom dbus badge-count code — not something to speculatively pull into this minimal bugfix PR before it's verified. I'll keep watching for a verified fix to land on master and merge it in once one does.

---

Generated by Claude Code

[wrench-exile-legacy]

It seems like the node version in the test.yaml workflow needs to be updated to at least node 22.

Mailspring/.github/workflows/test.yaml

Lines 32 to 34 in b22339b

	with:
	node-version: '20'
	cache: 'npm'

From the test workflow:

npm warn EBADENGINE Unsupported engine {
npm warn EBADENGINE   package: '@electron-internal/extract-zip@1.0.3',
npm warn EBADENGINE   required: { node: '>=22.12.0' },
npm warn EBADENGINE   current: { node: 'v20.20.2', npm: '10.8.2' }
npm warn EBADENGINE }