fix v0.3 spec: address 11 review issues (concurrency, codable, bcc, folder roles, etc.)

2026-03-13 22:17:49 +01:00
parent 5e1c26aa05
commit 8c3f51adb4
1 changed files with 68 additions and 13 deletions
--- a/docs/plans/2026-03-13-v0.3-compose-triage-design.md
+++ b/docs/plans/2026-03-13-v0.3-compose-triage-design.md
@@ -76,6 +76,7 @@ CREATE TABLE draft (
    forwardOf   TEXT,
    toAddresses TEXT,           -- JSON array of {"name", "address"}
    ccAddresses TEXT,           -- JSON array of {"name", "address"}
+    bccAddresses TEXT,          -- JSON array of {"name", "address"}
    subject     TEXT,
    bodyText    TEXT,
    createdAt   TEXT NOT NULL,
@@ -83,6 +84,14 @@ CREATE TABLE draft (
 );
 ```

+### Migration v2_mailboxRole: Special folder detection
+
+```sql
+ALTER TABLE mailbox ADD COLUMN role TEXT;  -- "trash", "archive", "sent", "drafts", "junk", or NULL
+```
+
+Populated during sync from LIST response attributes (RFC 6154). Falls back to name matching if no attributes are present.
+
 ### Migration v2_pendingAction: Offline action queue

 ```sql
@@ -124,10 +133,11 @@ public enum SMTPSecurity: String, Sendable, Codable {
    case starttls  // upgrade after connect, port 587
 }

-public struct OutgoingMessage: Sendable {
+public struct OutgoingMessage: Sendable, Codable {
    public var from: EmailAddress
    public var to: [EmailAddress]
    public var cc: [EmailAddress]
+    public var bcc: [EmailAddress]
    public var subject: String
    public var bodyText: String
    public var inReplyTo: String?     // Message-ID for threading
@@ -136,6 +146,11 @@ public struct OutgoingMessage: Sendable {
 }
 ```

+**Message-ID generation:** Generated at enqueue time by ComposeViewModel, before the action is persisted. Format: `<UUID@domain>` where domain is extracted from the sender's email address (e.g., `<550e8400-e29b-41d4-a716-446655440000@example.com>`). This ensures both the `send` and `append` actions reference the same Message-ID.
+
+```
+```
+
 ### SMTP Command Sequence

 ```
@@ -148,7 +163,7 @@ connect (TLS or plain)
 ← 235 authenticated
 → MAIL FROM:<sender>
 ← 250 OK
-→ RCPT TO:<recipient>  (repeat for each to + cc)
+→ RCPT TO:<recipient>  (repeat for each to + cc + bcc)
 ← 250 OK
 → DATA
 ← 354 go ahead
@@ -232,7 +247,7 @@ Detect well-known folders from mailbox LIST attributes (RFC 6154):
 | `\Drafts` | Drafts | "Drafts" |
 | `\Junk` | Spam | "Junk", "Spam" |

-Store detected folder names in the MailboxRecord for use by triage actions.
+Store the detected role in the `MailboxRecord.role` field (added in migration `v2_mailboxRole`). The ActionQueue uses `role` to resolve target folders for archive, delete, and sent mail operations without hardcoding folder names.

 ### Testing

@@ -252,16 +267,23 @@ Lives in the `SyncEngine` module. The central dispatcher for all write operation
 ### Design

 ```swift
-@MainActor
-public final class ActionQueue {
+public actor ActionQueue {
    private let store: MailStore
    private let imapClient: any IMAPClientProtocol
    private let smtpClient: SMTPClient?

-    /// Enqueue an action — applies locally first, then attempts remote dispatch
+    /// Enqueue an action — applies locally first, then attempts remote dispatch.
+    /// The local SQLite update (phase 1) is synchronous. The remote dispatch (phase 2)
+    /// runs as a detached Task on the actor's executor, not the main actor.
    public func enqueue(_ action: PendingAction) throws

-    /// Flush all pending actions to server (called by SyncCoordinator before fetch)
+    /// Enqueue multiple actions in a single SQLite transaction (e.g., send + append).
+    /// Actions are dispatched in array order.
+    public func enqueue(_ actions: [PendingAction]) throws
+
+    /// Flush all pending actions to server (called by SyncCoordinator before fetch).
+    /// Dispatches sequentially in creation order. Network I/O runs on the actor's
+    /// executor, not the main thread.
    public func flush() async

    /// Number of pending actions (for UI badge/indicator)
@@ -269,6 +291,8 @@ public final class ActionQueue {
 }
 ```

+**Concurrency note:** ActionQueue is a plain `actor`, not `@MainActor`. The `enqueue()` method writes to SQLite (fast, no network). The `flush()` method performs network I/O via SMTPClient/IMAPClient actors. ViewModels call `await actionQueue.enqueue(...)` from `@MainActor` — the hop to the ActionQueue actor is fine for the brief SQLite write. The GRDB ValueObservation on the main actor picks up the local changes immediately after `enqueue` returns.
+
 ### PendingAction Types

 ```swift
@@ -297,6 +321,18 @@ public enum ActionPayload: Sendable, Codable {
 }
 ```

+**Storage encoding:** `actionType` is derived from `payload` on insert (the enum discriminator). The `payload` column stores JSON using Swift's auto-synthesized `Codable` with explicit `CodingKeys` for each associated-value case. JSON shape per type:
+
+```json
+{"setFlags": {"uid": 42, "mailbox": "INBOX", "add": ["\\Seen"], "remove": []}}
+{"move": {"uid": 42, "from": "INBOX", "to": "Archive"}}
+{"delete": {"uid": 42, "mailbox": "INBOX", "trashMailbox": "Trash"}}
+{"send": {"message": { ... OutgoingMessage fields ... }}}
+{"append": {"mailbox": "Sent", "messageData": "...", "flags": ["\\Seen"]}}
+```
+
+The `actionType` column is redundant with the JSON discriminator but kept for efficient SQL queries (e.g., `WHERE actionType = 'send'`) without JSON parsing.
+
 ### Two-Phase Execution

 When the user performs an action:
@@ -322,7 +358,7 @@ When the user performs an action:
 | Error Type | Behavior |
 |-----------|----------|
 | Network timeout / connection refused | Retry on next flush. Exponential backoff: 30s, 1m, 2m, 4m, 8m, 15m cap. |
-| Auth failure (401/403) | Stop queue, surface re-auth prompt to user |
+| Auth failure (SMTP 535, IMAP NO on LOGIN) | Stop queue, surface re-auth prompt to user |
 | Permanent failure (message doesn't exist) | Mark failed, remove from queue, surface to user |
 | Retry cap exceeded (5 attempts) | Mark failed, surface to user |

@@ -348,6 +384,7 @@ This ensures the server reflects local changes before we pull new state.
 final class ComposeViewModel {
    var to: String = ""
    var cc: String = ""
+    var bcc: String = ""
    var subject: String = ""
    var bodyText: String = ""
    var mode: ComposeMode = .new
@@ -370,6 +407,14 @@ enum ComposeMode: Sendable {
 }
 ```

+### Body Fetch Requirement
+
+Reply and forward modes require the original message body for quoting. `MessageSummary.bodyText` may be nil if the body hasn't been fetched yet (v0.2 lazily fetches bodies). Before opening compose in reply/forward mode, the ViewModel must:
+
+1. Check if `bodyText` is populated
+2. If not, fetch the body via `IMAPClient.fetchBody(uid:)` and store it via `MailStore.storeBody()`
+3. If the fetch fails (offline), open compose without quoted text and show a note: "Original message body unavailable offline"
+
 ### Reply Behavior

 - **To:** Original sender (reply) or all recipients minus self (reply-all)
@@ -395,7 +440,7 @@ enum ComposeMode: Sendable {

 ### Send Sequence

-Two actions enqueued atomically:
+Two actions enqueued atomically via `ActionQueue.enqueue([send, append])` (single SQLite transaction):

 1. `send` — SMTPClient delivers the message
 2. `append` — IMAPClient appends the formatted message to the Sent folder with `\Seen` flag
@@ -413,7 +458,7 @@ If send succeeds but append fails, the append stays in the queue for retry. The

 - **macOS:** New window via `openWindow`
 - **iOS:** Sheet presentation
- Fields: To, CC (expandable), Subject, body TextEditor
+- Fields: To, CC, BCC (expandable), Subject, body TextEditor
 - Toolbar: Send button, Discard button
 - Compose is modal — one compose at a time in v0.3

@@ -426,7 +471,7 @@ If send succeeds but append fails, the append stays in the queue for retry. The
 | Action | IMAP Operation | Local Effect | macOS Shortcut |
 |--------|---------------|-------------|----------------|
 | Archive | MOVE to Archive folder | Update mailboxId | `e` |
-| Delete | MOVE to Trash | Update mailboxId | `⌫` |
+| Delete | MOVE to Trash (or EXPUNGE if already in Trash) | Update mailboxId (or remove record) | `⌫` |
 | Flag/unflag | STORE +/-\Flagged | Toggle isFlagged | `s` |
 | Read/unread | STORE +/-\Seen | Toggle isRead | `⇧⌘U` |
 | Move to folder | MOVE to chosen folder | Update mailboxId | `⌘⇧M` |
@@ -457,7 +502,7 @@ All actions go through the ActionQueue for offline safety.

 ### Thread-Level vs Message-Level

-Triage actions operate on the **entire thread** — all messages in the thread are moved/flagged together. This matches the standard email client behavior for threaded views.
+Triage actions operate on **all messages in the thread that belong to the currently selected mailbox**. A thread may span multiple mailboxes (e.g., a message in Inbox and its reply in Sent). Archiving from Inbox moves only the Inbox copies to Archive — the Sent copies stay in Sent. This matches standard email client behavior (Apple Mail, Gmail).

 ---

@@ -482,15 +527,25 @@ SMTP fields are optional for backwards compatibility with v0.2 accounts.

 ### Auto-Discovery Extension

-The existing `AutoDiscovery.parseISPDBXML` already receives the full XML. Extend it to also extract the `<outgoingServer type="smtp">` block:
+AutoDiscovery lives in the App target (`Apps/MagnumOpus/Services/AutoDiscovery.swift`) and stays there — it's UI-layer configuration logic, not core protocol code.
+
+The existing `AutoDiscovery.discoverIMAP(for:)` is renamed to `AutoDiscovery.discover(for:)` and returns a `DiscoveredConfig` containing both IMAP and SMTP settings:

 ```swift
 struct DiscoveredConfig: Sendable {
    var imap: DiscoveredServer?
    var smtp: DiscoveredServer?
 }
+
+enum AutoDiscovery {
+    /// Discover both IMAP and SMTP settings for an email address.
+    /// Replaces the v0.2 discoverIMAP(for:) method.
+    static func discover(for email: String) async -> DiscoveredConfig
+}
 ```

+The existing `parseISPDBXML` already receives the full XML. Extend it to also extract the `<outgoingServer type="smtp">` block — the XML structure is identical to the incoming server block. The DNS SRV fallback adds `_submission._tcp` lookup for SMTP alongside `_imaps._tcp` for IMAP.
+
 ### AccountSetupView Updates

 - Show SMTP fields alongside IMAP in manual mode