feat(email-service): Delay sending emails for undo functionality

[evanhutnik]

Summary

Add undo-send functionality by delaying email delivery through a configurable window.

Changes

Send Message Flow

• Before: The Send Message endpoint called Gmail's API directly to send messages immediately.
• After: Messages are queued to the existing scheduled message sender with a visibility delay (SENT_UNDO_DELAY_SECS), allowing users to undo the send before the pubsub worker processes it.

When users click "Undo" in the frontend, it calls the delete_scheduled_send endpoint, which converts the message back to a draft.

Configuration

• Added SENT_UNDO_DELAY_SECS environment variable (initially set to 5 seconds)

Infrastructure

• Removed FIFO designation from the scheduled send queue (FIFO queues don't support per-message delays, and ordering wasn't necessary)

Database

• Added processing boolean to email_scheduled_messages table to prevent race conditions when users attempt to unschedule a message that's actively being sent (cleaner than SELECT FOR UPDATE locks given potentially long send transactions with attachments)

API Changes

• Create Draft: Moved send_time outside of MessageToSend for cleaner logic (old field retained temporarily for backward compatibility)

Lambda Changes

• Scheduled Handler: Now filters for is_draft = true to exclude sent emails still within the undo window

[claude]

Code Review

I found one issue during the code review:

Missing check for processing flag in scheduled message processor

File: rust/cloud-storage/email_service/src/pubsub/scheduled/process.rs:57-70

The get_and_start_processing_scheduled_message function returns the OLD processing value before setting it to true. If another worker has already started processing this message, scheduled_message.processing will be true, but the code doesn't check this condition.

This creates a race condition where:

1. Worker A picks up a message and calls get_and_start_processing_scheduled_message, getting processing = false. The database is updated to processing = true.
2. Worker B picks up the same message (SQS standard queues allow duplicate delivery) and calls get_and_start_processing_scheduled_message, getting processing = true (the old value after A's update).
3. Worker B should skip processing since another worker is already handling it, but the current code only checks sent and send_time.
4. Both workers proceed to send the email, resulting in duplicate sends.

The test get_and_start_processing_returns_message_already_processing confirms the intended design is for callers to check the returned processing value.

Suggested fix:
Add a check for scheduled_message.processing between the sent check and the send_time check:

if scheduled_message.sent {
    tracing::warn!(
        message_id=%data.message_id,
        link_id=%data.link_id,
        "Scheduled message already sent - skipping"
    );
} else if scheduled_message.processing {
    tracing::warn!(
        message_id=%data.message_id,
        link_id=%data.link_id,
        "Scheduled message is already being processed by another worker - skipping"
    );
} else if scheduled_message.send_time > Utc::now() {
    tracing::warn!(
        message_id=%data.message_id,
        link_id=%data.link_id,
        send_time=scheduled_message.send_time.to_string(),
        "Scheduled message send_time is in the future - skipping"
    );
} else {
    // Process the message
}

[whutchinson98]

we probably want this higher no?

[evanhutnik]

increased to 5 seconds for now

[whutchinson98]

can you add a comment here that it defaults to 10 seconds

[whutchinson98]

please use the correct format