Skip to content

Surface JMAP error type when pin/add/remove labels fail#73

Merged
MadLlama25 merged 1 commit into
MadLlama25:mainfrom
Richyread:pr/surface-jmap-error-type
Jul 6, 2026
Merged

Surface JMAP error type when pin/add/remove labels fail#73
MadLlama25 merged 1 commit into
MadLlama25:mainfrom
Richyread:pr/surface-jmap-error-type

Conversation

@Richyread

Copy link
Copy Markdown
Contributor

When pinEmail, addLabels, or removeLabels gets back a JMAP notUpdated response, the caller sees a generic Failed to ... email with no indication of what JMAP actually rejected — the type and description fields on the notUpdated entry are read off the result shape and then discarded before the throw.

Through the MCP this surfaces as a bare JSON-RPC -32603 (Internal error: Tool execution failed: Failed to add labels to email.), with no way to distinguish a stateMismatch race against a concurrent delivery filter, vs. forbidden, vs. notFound, vs. serverFail. The operator sees a write failure they can't act on, and a downstream LLM agent has nothing to retry against.

The fix mirrors the pattern archiveEmail (jmap-client.ts:1170-1174) already uses: read err.type, append err.description if present, embed both in the thrown message.

Before:

Failed to add labels to email.

After:

Failed to add labels to email: stateMismatch
Failed to add labels to email: notFound - Email not found in this account
Failed to pin email: forbidden

The same defect pattern exists in markEmailRead, deleteEmail, moveEmail, and the bulk variants (bulkMarkRead, bulkPinEmails, bulkAddLabels, bulkRemoveLabels). Left for a follow-up so this change stays focused on the three handlers a real write failure surfaced through the MCP layer.

Test plan

  • npm run build clean
  • npm test — 176/176 pass. The existing notUpdated tests use assert.match with partial regex (e.g. /Failed to move/), so the appended : <type> suffix does not break them.
  • Manual reproduction on a fresh non-imported email: pin_email, add_labels, and remove_labels all succeed end-to-end with verified mailbox state changes — confirming the success path is unchanged.

This PR was drafted with Claude Opus 4.7; I reviewed and tested each commit.

When pinEmail, addLabels, or removeLabels gets back a JMAP `notUpdated`
response, the caller sees a generic "Failed to ... email" with no
indication of what JMAP actually rejected — the `type` and
`description` fields on the notUpdated entry are read off the result
shape and then discarded before the throw.

Through the MCP this surfaces as a bare JSON-RPC -32603 ("Internal
error: Tool execution failed: Failed to add labels to email."), with
no way to distinguish a stateMismatch race against a concurrent
delivery filter, vs. forbidden, vs. notFound, vs. serverFail. The
operator sees a write failure they can't act on, and a downstream LLM
agent has nothing to retry against.

The fix mirrors the pattern archiveEmail (jmap-client.ts:1170-1174)
already uses: read `err.type`, append `err.description` if present,
embed both in the thrown message.

Before:
  Failed to add labels to email.

After:
  Failed to add labels to email: stateMismatch
  Failed to add labels to email: notFound - Email not found in this account
  Failed to pin email: forbidden

The same defect pattern exists in markEmailRead, deleteEmail,
moveEmail, and the bulk variants (bulkMarkRead, bulkPinEmails,
bulkAddLabels, bulkRemoveLabels). Left for a follow-up so this change
stays focused on the three handlers a real write failure surfaced
through the MCP layer.

Verification:
- bun run build: clean
- bun run test: 196/196 pass. The existing notUpdated tests use
  assert.match with partial regex (e.g. /Failed to move/), so the
  appended `: <type>` suffix does not break them.
- Manual reproduction on a fresh non-imported email: pin_email,
  add_labels, and remove_labels all succeed end-to-end with verified
  mailbox state changes — confirming the success path is unchanged.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants