Skip to content

feat(email): mailbox folders, greylist review, and outbound drafts#97

Open
Jacky040124 wants to merge 10 commits into
mainfrom
feat/mail-greylist
Open

feat(email): mailbox folders, greylist review, and outbound drafts#97
Jacky040124 wants to merge 10 commits into
mainfrom
feat/mail-greylist

Conversation

@Jacky040124
Copy link
Copy Markdown
Collaborator

@Jacky040124 Jacky040124 commented May 22, 2026

Summary

  • Add an mailbox column on emails (inbox, sent, draft, untrust) with a D1 migration and shared query/helpers for folder-based listing and mailbox updates.
  • Replace hard SMTP rejection for non-whitelisted senders with a greylist flow: inbound mail is stored for review in Draft instead of being rejected at the worker.
  • Add outbound draft creation/sending with a send-state machine, discard-to-untrust for inbound review mail, and email UI updates (Draft folder, compose save/send, hydration fixes).

Test plan

  • pnpm typecheck
  • pnpm test
  • Apply migration 0033_email_mailbox.sql before deploy
  • Send from a non-whitelisted address and confirm it appears in Draft (not rejected)
  • Discard a review email and confirm it moves to Untrust
  • Create, save, and send an outbound draft from the email page
  • Verify inbox/sent/draft/untrust folders list the expected messages

Made with Cursor

Jacky040124 and others added 5 commits May 22, 2026 10:10
Introduce mailbox classification on emails with migration, shared
constants, query helpers, and draft-send lifecycle functions.

Co-authored-by: Cursor <cursoragent@cursor.com>
Stop rejecting unknown senders at the worker; store them for review
via notify and update whitelist empty-state copy.

Co-authored-by: Cursor <cursoragent@cursor.com>
Route folder queries through mailbox helpers and support discarding
inbound review emails into the untrust mailbox.

Co-authored-by: Cursor <cursoragent@cursor.com>
Add draft CRUD/send APIs, extract sender resolution, and wire compose
plus client helpers for saving and sending drafts.

Co-authored-by: Cursor <cursoragent@cursor.com>
Add draft folder UX, discard/send-from-draft actions, race-safe loading
state helpers, and hydration shells for sidebar and email page.

Co-authored-by: Cursor <cursoragent@cursor.com>
@Jacky040124 Jacky040124 requested a review from a team as a code owner May 22, 2026 17:12
Align folder filtering assertions with mailbox columns: untrust inserts
set mailbox explicitly, inbox checks mailbox, and draft covers greylist
review emails.

Co-authored-by: Cursor <cursoragent@cursor.com>
Copy link
Copy Markdown
Contributor

@gusye1234 gusye1234 left a comment

Choose a reason for hiding this comment

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

Review Summary

Architecture is solid — approve with minor suggestions.

The mailbox column + greylist flow + draft send state machine are well-designed. Test coverage is thorough.

Minor Items (non-blocking):

  1. Silent error swallowing in send routerestoreClaimedDraft and markClaimedDraftUnknown functions swallow errors. If those DB calls fail, the draft stays stuck in sending state forever. Consider logging failed restore/markUnknown calls.

  2. Send route length (~400 lines) — The draft state machine layered on top of direct-send has grown the file significantly. Future refactor splitting draft-send from direct-send would improve maintainability.

  3. Migration gap — Emails with direction = 'outbound' but status != 'sent' (failed old sends) will get mailbox = 'inbox' by default. Minor data integrity gap, unlikely to cause user-visible issues.

  4. NaminghandleDiscardDraft operates on inbound review emails, not user-composed drafts. Could mislead future devs. Consider handleDiscardReviewEmail.

None of these block merge.

Jacky040124 and others added 4 commits May 25, 2026 19:54
Route non-whitelisted inbound mail through a fail-closed triage task, and apply triage results through scoped mailbox state transitions so user-handled mail is not overwritten.

Co-authored-by: Cursor <cursoragent@cursor.com>
Run email triage with a dedicated readonly execution profile across supported providers, and fail closed when a provider cannot enforce the boundary.

Co-authored-by: Cursor <cursoragent@cursor.com>
Add a local end-to-end script that exercises the main email triage branches and preserves test data for UI verification.

Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Copy link
Copy Markdown
Contributor

@gusye1234 gusye1234 left a comment

Choose a reason for hiding this comment

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

Follow-up Review — New Commits (e49154d, a2fe1ab, bbc0435)

Reviewing the readonly triage workflow added since my last review on 2026-05-23.

What was added

Routes non-whitelisted inbound email through a fail-closed readonly agent triage session. The agent classifies email as untrust or draft_reply via structured JSON output. Multiple enforcement layers: Claude permission-mode denial, Codex read-only sandbox, OpenCode fail-closed, plus prompt-level instructions.

Findings

Major — Rate limiting on triage task creation (non-blocking)

Every non-whitelisted inbound email spawns a full LLM agent session. A spam flood could overwhelm the task queue and burn LLM credits. No deduplication or queue-depth guard exists before enqueue.

Recommendation: Add a guard checking total queued triage tasks before enqueuing, or batch triage for low-priority senders. Can be a follow-up ticket.

Minor — recoverStaleEmailTriageApplies uses emails.createdAt not apply-start time

The 1-hour staleness check compares against createdAt (when the email arrived), not when triage-apply started. If an email sat in draft before triage ran, it could be recovered prematurely. Unlikely in practice since triage is enqueued immediately, but semantically off. Consider using updatedAt or checking the triage task's completion time.

Minor — enqueueEmailTriageTask returns null silently on multiple failure paths

When triage can't be enqueued (R2 unavailable, empty body), the email is orphaned in draft with no logging of why. Consider a warning log.

Minor — Failed triage task leaves email in draft with no owner notification

Correct fail-closed behavior (don't auto-trust), but no mechanism alerts the owner that manual review is needed. Consider a dashboard indicator or notification.

Security Assessment

The readonly enforcement is defense-in-depth and sound for a first iteration:

  • Claude: control_requests denied + no CLI tool docs injected in triage context
  • Codex: OS-level read-only sandbox (CLI) / approval rejection (MCP)
  • OpenCode: fails closed entirely (rejects triage tasks)
  • Fail-closed on every error path — invalid output, crashes, timeouts, DB failures all leave email in draft

The practical attack surface for bypassing readonly requires the LLM to spontaneously construct the exact CLI command without being told about it — acceptable risk given controlled workspace.

Verdict

Solid implementation. The fail-closed principle is genuinely maintained across all failure paths. Tests are comprehensive. Approve with the rate-limiting concern tracked as a follow-up.

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