diff --git a/Companion/Decisions/V3/Active/adr-ios-080.md b/Companion/Decisions/V3/Active/adr-ios-080.md new file mode 100644 index 00000000..a0e109f7 --- /dev/null +++ b/Companion/Decisions/V3/Active/adr-ios-080.md @@ -0,0 +1,87 @@ +## ADR-IOS-080: Body Indexing Is Bounded, Payload-Selective, and Truthfully Terminal + +**Date:** 2026-08-31 + +**Status:** Active. + +**Context.** An IMAP message part can be larger than SwiftMail's ordinary response parser limit. +Fetching the complete part therefore fails deterministically with `PayloadTooLargeError`; reducing +the number of messages in a batch cannot split that one literal. Affected rows have complete +headers but bodies that are neither indexed nor confirmed empty, so body queues stop making progress +while Fast Sync continues to count the rows as pending forever. +Increasing the response limit merely moves the failure threshold and raises memory use. Marking the +rows empty or complete would lie about server content that was never fetched. + +**Decision.** + +1. **IMAP MIME parts are fetched with validated partial `BODY.PEEK` ranges.** The SwiftMail fork + exposes `fetchPart(section:of:offset:count:)`, encodes + `BODY.PEEK[section]`, and accepts bytes only from the requested message identity, + exact section kind and origin. It rejects ignored/malformed ranges, impossible lengths, extra + body literals and responses larger than the requested count while draining the wire safely. +2. **The app fetches required parts in one-MiB chunks.** Transfer-encoded bytes are concatenated + before base64 or quoted-printable decoding, because encoded units can straddle a chunk boundary. + The metadata preflight explicitly requests ENVELOPE, INTERNALDATE, FLAGS and BODYSTRUCTURE; it + does not use SwiftMail's default full-header option because `BODY.PEEK[HEADER]` is itself an + unbounded literal and existing stored headers remain authoritative for non-ENVELOPE fields. + A BODYSTRUCTURE size is a planning hint, not truncation authority: after reaching it, the app + probes one byte at the advertised endpoint and accepts completion only when the server returns + an empty range. Where size is unavailable, a short nonempty response is not treated as EOF and + fetching continues until an empty range. The server-reported size does not trigger a full-size + allocation before bytes arrive. +3. **Background body indexing downloads only render ingredients.** + `IMAPFetchMapping.isRequiredBodyPart` selects visible `text/plain` and `text/html`, calendar + content, and non-attachment CID images. BODYSTRUCTURE metadata is sufficient for normal + attachments and opaque `.eml` payloads. Flattened descendants of an attached `message/rfc822` + part are excluded component-wise as attachment payload too. Attachment bytes are fetched on + demand through the same bounded path. Attached HTML is metadata, not a display body. +4. **The ordinary four-MiB response parser limit and NSE memory limit stay unchanged.** The bounded + transport is the fix; there is no larger-buffer fallback. The NSE uses the same payload selection + and chunking, never downloads ordinary attachment payloads, and declines active body rendering + before fetching when required BODYSTRUCTURE sizes are unknown or exceed its aggregate admission + budget. +5. **A server that cannot honor partial ranges creates an explicit terminal-unindexed state.** + `MessageHeader.bodyIndexingFailureReason = "partial_fetch_unsupported"` means the row is neither + indexed nor empty, but is no longer runnable by automatic body queues. Only deterministic partial + protocol/assembly failures take this path; transient connection and database failures remain + retryable. The write is guarded by the row's full provider address plus positive identity proof + from the failed fetch: its exact SELECT UIDVALIDITY epoch must match the stored row whenever both + epochs exist; only when epoch evidence is unavailable may a matching fetched RFC 5322 Message-ID + serve as fallback proof. A move, re-key, or reused UID therefore cannot attach the outcome to + another message, and a duplicate Message-ID cannot override an explicit epoch contradiction. +6. **Completion reports runnable work and truthful omissions separately.** `pendingBodyCount` + excludes terminal-unindexed rows; `unindexedBodyCount` reports them. Once the header walk and all + runnable body work finish, the UI says `Sync complete with N messages not indexed` and displays a + completed progress bar instead of holding indexing active forever. Smart Reindex clears terminal + reasons so a later app or server upgrade can try again. + +**Rationale.** A finite chunk bound closes the arbitrary-size-literal class without weakening the +parser's memory boundary. Payload selection avoids paying attachment memory and network cost during +background indexing. The terminal state separates three facts that must never be conflated: +indexed content, confirmed-empty content, and content that exists but cannot be indexed with the +server's current protocol behaviour. + +**Consequences.** + +- Large text/calendar/CID parts can be reconstructed beyond 32 MiB while every response literal + stays below the ordinary parser limit. +- Background indexing uses more IMAP commands for large required parts, in exchange for bounded + memory and deterministic progress. +- Normal attachment payloads are absent from background `MessagePart` values and are downloaded + only when requested through the same chunked path. BODYSTRUCTURE still supplies their filename, + MIME type, section and size; `.eml` attachments are parsed for preview only after that tap-time + download. +- A non-compliant server may leave messages unindexed, but the state is visible, terminal, and + retryable by explicit Smart Reindex rather than silently empty, falsely complete, or infinite. + +**Tests / evidence.** SwiftMail wire tests reconstruct a body larger than 32 MiB under a four-MiB +parser buffer and cover ignored/malformed ranges, wrong identity/section/origin, oversized and +duplicate literals, UID ordering, and unsolicited FETCH updates. iOS tests cover encoded-boundary +assembly, oversized raw-header exclusion, understated and unknown BODYSTRUCTURE sizes, payload +selection on the provider's wire hot path, on-demand `.eml` fetching, NSE admission/state +transitions, identity-safe terminal writes, +queue convergence, migration/default state, queue exclusion, Smart Reindex recovery, and exact +completion wording. + +**Relates:** ADR-IOS-050 (`bodyComplete` is FTS truth), ADR-IOS-072 (content ownership), +ADR-IOS-075 (acknowledge only committed cache state), bounded-memory absolute. diff --git a/Companion/Memory/Current/030-backfill-fast-sync-completion-gate-on-pendingbodycount-never-a-server-to.md b/Companion/Memory/Current/030-backfill-fast-sync-completion-gate-on-pendingbodycount-never-a-server-to.md index b0ce2eb6..41fac7ec 100644 --- a/Companion/Memory/Current/030-backfill-fast-sync-completion-gate-on-pendingbodycount-never-a-server-to.md +++ b/Companion/Memory/Current/030-backfill-fast-sync-completion-gate-on-pendingbodycount-never-a-server-to.md @@ -1,3 +1,13 @@ + +> **Current amendment (2026-08-31, ADR-IOS-080):** `pendingBodyCount` now selects +> `headerComplete=1 AND bodyComplete=0 AND bodyEmptyConfirmed=0 AND +> bodyIndexingFailureReason IS NULL`. A deterministic failure to honor validated partial IMAP +> ranges is persisted as terminal-unindexed, counted separately by `unindexedBodyCount`, and shown +> as `Sync complete with N messages not indexed` once runnable work drains. Such rows are never +> confirmed empty and never marked indexed; Smart Reindex clears the reason for an explicit retry. +> This supersedes the preserved body's claim that every eligible row is self-terminating through +> success/empty and its reference to oversized bodies as confirmed-empty. + ### Backfill / Fast Sync Completion — gate on `pendingBodyCount`, NEVER a server total - **`BackfillProgress.isFullyComplete` gates on `headersDone && totalEmails > 0 && pendingBodyCount == 0`** (`AccountManager.swift`). `pendingBodyCount` = body-eligible headers still awaiting fetch (`headerComplete=1 AND bodyComplete=0 AND bodyEmptyConfirmed=0`), the same criteria `BackfillBodyQueue`/`ActiveBodyQueue` select on. It is local and self-terminating (empty/404/oversized bodies confirm-empty), so it reaches 0 once the body queues have nothing fetchable left. diff --git a/Companion/Memory/Current/065-screen-keep-awake-chat-pill.md b/Companion/Memory/Current/065-screen-keep-awake-chat-pill.md index 21e2d167..04b3ace6 100644 --- a/Companion/Memory/Current/065-screen-keep-awake-chat-pill.md +++ b/Companion/Memory/Current/065-screen-keep-awake-chat-pill.md @@ -2,4 +2,5 @@ ### Screen Keep-Awake (chat pill) - `Theme/ScreenKeepAwake.swift` — reference-counted wrapper around `UIApplication.isIdleTimerDisabled` + `.keepScreenAwake(while:)` view modifier. The idle-timer flag is a single global, so holders are counted; the modifier tracks its own held state (`@State holding`) so acquire/release fire exactly once per transition regardless of `onChange` vs `onDisappear` teardown ordering. - Applied once in `DynamicIslandChatButton.swift` on the pill root: `isExpanded || isWorking || ActiveAgentTracker.shared.anyWorking || speechRecognizer.isRecording` (same scope as the wand-glow indicator). Covers all three host screens (Inbox/Compose/MessageDetail) with no per-screen wiring. +- `FastSyncView` passes `true` for its entire presentation lifetime. This is intentionally video-playback style: header/body progress, queue idleness, and completion do not release the hold while the view remains open; the modifier releases it on view disappearance. - No `scenePhase` handling needed — iOS only honors `isIdleTimerDisabled` while the app is foreground; the hold resumes automatically on return. Reuse `.keepScreenAwake(while:)` for any future keep-awake need (never set `isIdleTimerDisabled` directly). diff --git a/DECISIONS.md b/DECISIONS.md index bc3779aa..7cb0aa17 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -127,7 +127,7 @@ These records were authored after `v1.6.38`, so the pinned compaction has no byt - **[ADR-IOS-026B — the v3 supersession record](Companion/Decisions/V3/Superseded/adr-ios-026b-v3-superseded-by-068.md)** — *PendingOperation Uses Stable IDs (rfc822MessageId)*, **SUPERSEDED 2026-08-02 by ADR-IOS-068** and retained verbatim as evidence: `MessageHeader.stableId`, `IMAPProvider.resolveUID`'s Message-ID `SEARCH`, dual-match pending-op filtering, the UIDVALIDITY rationale. **Only its durable-mutation-authority layer is superseded** — fetch, normalize, dedup, stage, the AI cross-device cache probe, threading/`References`, and Outbox send de-duplication all SURVIVE; ADR-IOS-068's exempt list is normative. Authored under the colliding number `ADR-IOS-026`, so both search terms find it. The byte-identical `v1.6.38` twin, without the supersession banner, is [`Companion/Decisions/Superseded/adr-ios-026b.md`](Companion/Decisions/Superseded/adr-ios-026b.md). - **[Compaction drift list](Companion/Decisions/V3/retained-inline-no-byte-identical-routed-twin.md)** — the retired *Retained inline — no byte-identical routed twin* preamble: check the routed twin before editing a post-`v1.6.38` amendment. -# v3 records (ADR-IOS-068 … 076) +# v3 records (ADR-IOS-068 … 080) > **Numbering note.** This file jumps from ADR-IOS-057 to ADR-IOS-068. That gap is deliberate and is > itself a record: **ADR-IOS-058, 059, 060, 061, 062, 063, 064, 065, 066 and 067 were authored on a @@ -149,3 +149,4 @@ These records were authored after `v1.6.38`, so the pinned compaction has no byt - **[ADR-IOS-077](Companion/Decisions/V3/Active/adr-ios-077.md)** — Active. Hostile attachment filenames are **REJECTED, not reduced** (`c35cfdca2`, net −476): one shared `AttachmentFilename.isSafeFileComponent` predicate, throw before `createDirectory` on save and refuse before the fetch on download, generic `"Unsupported file name"` for all six rules. Reducer + co-edit twin DELETED — all five confirmed defects lived in the *transformation*, none in the classification. ⚠️ **Rejecting at save does NOT make the loaders safe** — `metaBase`/`afterIndexPrefix` stay load-bearing; type-spoof is bounded, not closed; the combining test is `ccc != 0` on NFD, **not** category `Mn`/`Mc`/`Me`. ⚠️ **Consequence 5 retracts the MIGRATION GUARANTEE — there was never a reducer to migrate FROM** (`v1.7.6`/`v1.7.7`/`v1.7.8` write the name verbatim, so legacy on-disk names are RAW sender-authored; stranded set = refused ∩ writable-by-v1.7.8, 3 narrow shapes). `IOS-ATTACH-001` — forward-only by owner verdict: **no migration, rename-on-load or grandfathering path.** Pre-compaction bullet, byte-for-byte: [pre-compaction-index-lines.md](Companion/Decisions/V3/pre-compaction-index-lines.md). - **[ADR-IOS-078](Companion/Decisions/V3/Active/adr-ios-078.md)** — Active. Newest-100 bounds sync-origin AI processing only; existing summaries always display, while action tags remain Inbox-only. `ActiveAIQueue.recentInboxWindowContains`, `AIJob.windowExempt`, `MIS-IOS-018`, #68. [Prior catalog wording](Companion/Decisions/V3/pre-compaction-index-lines-078-079.md#source-line-150--adr-ios-078) - **[ADR-IOS-079](Companion/Decisions/V3/Active/adr-ios-079.md)** — Active. Scheduled tasks and `taskCache` are deleted from iOS, remain live on Thunderbird; `[Task]` prose and `disabledReminders` `t:` hashes are retained. [Prior catalog wording](Companion/Decisions/V3/pre-compaction-index-lines-078-079.md#source-line-151--adr-ios-079) +- **[ADR-IOS-080](Companion/Decisions/V3/Active/adr-ios-080.md)** — Active. IMAP body indexing uses validated bounded partial fetches, skips normal attachment payloads in background, and reports terminal-unindexed rows as `Sync complete with N messages not indexed` instead of retrying forever. diff --git a/PROJECT_MEMORY.md b/PROJECT_MEMORY.md index c051a5a8..f2deb81a 100644 --- a/PROJECT_MEMORY.md +++ b/PROJECT_MEMORY.md @@ -82,7 +82,7 @@ Search the topic text below as subsystem keywords. Each link is mandatory when i | Manual Tag Teaching (Long-Press Context Menu) | [read in full](Companion/Memory/Current/062-manual-tag-teaching-long-press-context-menu.md) | | Tool Registry (Scaffold) | [read in full](Companion/Memory/Current/063-tool-registry-scaffold.md) | | Agent Chat (ADR-IOS-022, ADR-IOS-023) | [read in full](Companion/Memory/Current/064-agent-chat-adr-ios-022-adr-ios-023.md) | -| Screen Keep-Awake (chat pill) | [read in full](Companion/Memory/Current/065-screen-keep-awake-chat-pill.md) | +| Screen Keep-Awake (chat pill + Fast Sync full-view hold) | [read in full](Companion/Memory/Current/065-screen-keep-awake-chat-pill.md) | | Agent Compose FIFO Queue (ADR-IOS-030) | [read in full](Companion/Memory/Current/066-agent-compose-fifo-queue-adr-ios-030.md) | | Outgoing Threading — reply/forward stay in-thread on every provider (ADR-IOS-043, 2026-06-23) | [read in full](Companion/Memory/Current/067-outgoing-threading-reply-forward-stay-in-thread-on-every-provider-adr-io.md) | | Incoming Thread Detection — `ThreadDetection.findRelatedMessages` has NO subject-based fallback (by design) | [read in full](Companion/Memory/Current/068-incoming-thread-detection-threaddetection-findrelatedmessages-has-no-sub.md) | diff --git a/Shared/Parse/IMAPFetchMapping.swift b/Shared/Parse/IMAPFetchMapping.swift index 349cbc16..8b633652 100644 --- a/Shared/Parse/IMAPFetchMapping.swift +++ b/Shared/Parse/IMAPFetchMapping.swift @@ -5,6 +5,14 @@ import Foundation import SwiftMail +enum IMAPPartialFetchAssemblyError: Error, Equatable, Sendable { + case invalidExpectedSize(Int) + case invalidChunkSize(Int) + case chunkExceedsRequest(requested: Int, received: Int) + case prematureEnd(expected: Int, received: Int) + case contentBeyondExpectedSize(expected: Int) +} + /// Pure helpers shared between the NSE's one-shot IMAP fetch and the main-app /// IMAP pipeline. Extracted into Shared/ so they compile into BOTH the TabMail /// and TabMailNotificationService targets, and are reachable from TabMailTests. @@ -31,6 +39,192 @@ enum IMAPFetchMapping { /// upstream mirror and the value lives here at the call sites instead. static let responseBufferLimit = 4 * 1024 * 1024 + /// One MiB keeps each literal comfortably below the ordinary four-MiB + /// response parser limit while avoiding excessive command overhead. + static let bodyPartChunkSize = 1024 * 1024 + + /// Metadata required to render/index a message body without requesting the + /// unbounded raw `BODY.PEEK[HEADER]` literal included by SwiftMail's + /// `.default` options. ENVELOPE carries the identity/address fields used by + /// body processing; BODYSTRUCTURE supplies the MIME tree and attachment + /// metadata. Existing stored headers remain authoritative for fields such + /// as References that are not part of ENVELOPE. + static let bodyFetchMetadataOptions: FetchMessageInfoOptions = [ + .envelope, .internalDate, .flags, .bodyStructure, + ] + + /// BODYSTRUCTURE is enough for normal attachment rows. Background body + /// work downloads only render ingredients: visible text, calendar data, and + /// CID images. File attachments are fetched on demand. + static func isRequiredBodyPart(_ part: MessagePart) -> Bool { + let contentType = part.contentType.lowercased() + let disposition = part.disposition?.lowercased() + if contentType.hasPrefix("text/calendar") { return true } + if (contentType.hasPrefix("text/plain") || contentType.hasPrefix("text/html")) + && !isNormalAttachment(part) { + return true + } + return contentType.hasPrefix("image/") + && part.contentId != nil + && disposition != "attachment" + } + + /// BODYSTRUCTURE flattens the children of `message/rfc822` parts into the + /// same array. A child text or CID part therefore has no disposition of its + /// own that reveals it belongs to an attached message. Exclude descendants + /// of normal attached messages component-wise; explicitly inline embedded + /// messages may still contribute render ingredients. + static func requiredBodyPartIndices(in parts: [MessagePart]) -> [Int] { + let attachedMessageSections = parts.compactMap { part -> [Int]? in + guard part.contentType.lowercased().hasPrefix("message/rfc822"), + isNormalAttachment(part) else { return nil } + return part.section.components + } + return parts.indices.filter { index in + let components = parts[index].section.components + let belongsToAttachedMessage = attachedMessageSections.contains { parent in + components.count > parent.count + && Array(components.prefix(parent.count)) == parent + } + return !belongsToAttachedMessage && isRequiredBodyPart(parts[index]) + } + } + + /// Whether every background-render part has a known BODYSTRUCTURE size and + /// their encoded-octet total fits a caller's aggregate memory budget. + /// The NSE uses this before allocating any body literal: one-MiB wire chunks + /// bound the parser, but retaining and rendering all chunks still needs a + /// separate whole-message bound inside its fixed process budget. + static func requiredBodyPartsFitAggregateBudget( + in parts: [MessagePart], + byteBudget: Int + ) -> Bool { + guard byteBudget >= 0 else { return false } + var total = 0 + for index in requiredBodyPartIndices(in: parts) { + guard let size = parts[index].size, size >= 0, + size <= byteBudget - total else { return false } + total += size + } + return true + } + + /// Admission plus bounded fetch orchestration for the NSE. Keeping the + /// aggregate check and chunk loop in one shared, injectable operation + /// prevents the extension from admitting with one policy and then fetching + /// through an unbounded path. `nil` means passive-delivery fallback and + /// guarantees `fetchChunk` was never called. + static func fetchRequiredBodyPartsWithinAggregateBudget( + in parts: [MessagePart], + byteBudget: Int, + fetchChunk: (_ part: MessagePart, _ offset: Int, _ count: Int) async throws -> Data + ) async throws -> [MessagePart]? { + guard requiredBodyPartsFitAggregateBudget(in: parts, byteBudget: byteBudget) else { + return nil + } + var fetchedParts = parts + for index in requiredBodyPartIndices(in: fetchedParts) { + let part = fetchedParts[index] + fetchedParts[index].data = try await concatenateEncodedPart( + expectedSize: part.size + ) { offset, count in + try await fetchChunk(part, offset, count) + } + } + return fetchedParts + } + + private static func isNormalAttachment(_ part: MessagePart) -> Bool { + let disposition = part.disposition?.lowercased() + let hasFilename = !(part.filename?.isEmpty ?? true) + return disposition == "attachment" || (hasFilename && disposition != "inline") + } + + /// Concatenate transfer-encoded bytes first; callers decode once afterwards. + /// Decoding each chunk independently would corrupt base64 and quoted-printable + /// sequences that straddle a chunk boundary. + static func concatenateEncodedPart( + expectedSize: Int?, + chunkSize: Int = bodyPartChunkSize, + fetchChunk: (_ offset: Int, _ count: Int) async throws -> Data + ) async throws -> Data { + guard chunkSize > 0 else { + throw IMAPPartialFetchAssemblyError.invalidChunkSize(chunkSize) + } + if let expectedSize, expectedSize < 0 { + throw IMAPPartialFetchAssemblyError.invalidExpectedSize(expectedSize) + } + var result = Data() + if let expectedSize { result.reserveCapacity(min(expectedSize, chunkSize)) } + var offset = 0 + + while true { + if let expectedSize, offset >= expectedSize { + // BODYSTRUCTURE is useful planning metadata, not authority for + // truncation. Prove EOF with one bounded request at its claimed + // endpoint so an understated (including zero) size cannot be + // cached as a complete/empty body. + let extra = try await fetchChunk(offset, 1) + guard extra.count <= 1 else { + throw IMAPPartialFetchAssemblyError.chunkExceedsRequest( + requested: 1, received: extra.count + ) + } + guard extra.isEmpty else { + throw IMAPPartialFetchAssemblyError.contentBeyondExpectedSize( + expected: expectedSize + ) + } + break + } + let requested = expectedSize.map { min(chunkSize, $0 - offset) } ?? chunkSize + let chunk = try await fetchChunk(offset, requested) + guard chunk.count <= requested else { + throw IMAPPartialFetchAssemblyError.chunkExceedsRequest( + requested: requested, received: chunk.count + ) + } + + if chunk.isEmpty { + try validatePartialEnd(expectedSize: expectedSize, received: offset) + break + } + + result.append(chunk) + offset += chunk.count + // RFC partial FETCH count is a maximum. A short non-empty response + // still advances the origin; it is not proof of end-of-section. + // Known BODYSTRUCTURE sizes terminate at the loop guard. Unknown- + // size callers terminate only when the server returns an empty range. + } + return result + } + + private static func validatePartialEnd(expectedSize: Int?, received: Int) throws { + guard let expectedSize, received != expectedSize else { return } + throw IMAPPartialFetchAssemblyError.prematureEnd( + expected: expectedSize, + received: received + ) + } + + static func isDeterministicPartialFetchFailure(_ error: Error) -> Bool { + if let partialError = error as? PartialFetchError { + switch partialError { + case .messageNotFound, .invalidRange: + return false + default: + return true + } + } + return error is IMAPPartialFetchAssemblyError + || isResponseBufferOverflow(error) + } + + static func isResponseBufferOverflow(_ error: Error) -> Bool { + String(describing: error).contains("PayloadTooLargeError") + } + /// Build the `messageId` string used as `MessageHeader.messageId`. /// /// MUST match `IMAPProvider.buildMessageHeaderInfo`'s format so rows the @@ -135,7 +329,7 @@ enum IMAPFetchMapping { filename: filename, contentType: part.contentType, section: part.section.description, - size: part.data?.count ?? 0, + size: part.size ?? part.data?.count ?? 0, encoding: part.encoding ) } @@ -144,8 +338,8 @@ enum IMAPFetchMapping { // Server-parsed `message/rfc822` parts already have their children // visible at the top level (BODYSTRUCTURE exposes them at numeric // sub-sections like `2.1`, caught above). File-uploaded `.eml`s are - // opaque blobs server-side — the nested attachments only exist - // after we parse the bytes ourselves. `encoding` on each nested + // opaque blobs server-side, and their nested attachments are available + // only when a caller supplied the parent bytes on demand. `encoding` on each nested // entry is set to the PARENT's transfer encoding so tap-time // resolution can re-fetch parent bytes with the right encoding. for part in message.parts where EmlParsing.isEmlFilename(part.filename) @@ -167,31 +361,44 @@ enum IMAPFetchMapping { return out } - /// Inline image extraction from a fetched message. Mirrors - /// `IMAPProvider.buildFullMessageInfo`'s CID loop — uses - /// `message.cids.prefix(maxInlineImages)` with `decodedData()` to - /// handle base64/quoted-printable transfer encoding before the - /// renderer re-encodes as a `data:` URI. Strips angle brackets + - /// whitespace from the Content-ID. - static func extractInlineImages(message: Message, maxInlineImages: Int) -> [InlineImageRef] { - message.cids.prefix(maxInlineImages).compactMap { part in + /// Inline image extraction shared by the app and NSE. Metadata-only or + /// ineligible CIDs are filtered before applying the cap so an attachment + /// cannot consume a slot needed by a fetched render ingredient. + static func extractInlineImages( + message: Message, + maxInlineImages: Int, + eligibleSections: Set? = nil + ) -> [InlineImageRef] { + Array(message.cids.lazy.compactMap { part -> InlineImageRef? in + if let eligibleSections, + !eligibleSections.contains(part.section.description) { + return nil + } guard let rawId = part.contentId, let data = part.decodedData() else { return nil } let contentId = rawId.trimmingCharacters(in: .whitespacesAndNewlines) .trimmingCharacters(in: CharacterSet(charactersIn: "<>")) .trimmingCharacters(in: .whitespacesAndNewlines) guard !contentId.isEmpty else { return nil } return InlineImageRef(contentId: contentId, contentType: part.contentType, data: data) - } + }.prefix(max(0, maxInlineImages))) } /// First `text/calendar` part's decoded bytes, if any. Mirrors /// `IMAPProvider.buildFullMessageInfo`'s ICS-data extraction — lets /// the renderer skip calling `attachmentFetcher` for invite bodies /// when we already have the bytes in memory from the batch fetch. - static func extractICSData(message: Message) -> Data? { - message.parts.first(where: { - $0.contentType.lowercased().contains("text/calendar") - })?.decodedData() + static func extractICSData( + message: Message, + eligibleSections: Set? = nil + ) -> Data? { + message.parts.lazy.compactMap { part -> Data? in + guard part.contentType.lowercased().contains("text/calendar") else { return nil } + if let eligibleSections, + !eligibleSections.contains(part.section.description) { + return nil + } + return part.decodedData() + }.first } /// Convert a fetched IMAP `(info, message)` pair into the canonical @@ -230,7 +437,8 @@ enum IMAPFetchMapping { $0.contentType.lowercased().hasPrefix("message/rfc822") ? $0.section.components : nil } return info.parts.compactMap { part in - guard part.contentType.lowercased().hasPrefix("text/html") else { return nil } + guard isRequiredBodyPart(part), + part.contentType.lowercased().hasPrefix("text/html") else { return nil } let comp = part.section.components let nested = rfc822Sections.contains { rfc in comp.count > rfc.count && Array(comp.prefix(rfc.count)) == rfc diff --git a/TabMail/Models/MessageHeader.swift b/TabMail/Models/MessageHeader.swift index 640fb564..fe90256a 100644 --- a/TabMail/Models/MessageHeader.swift +++ b/TabMail/Models/MessageHeader.swift @@ -5,6 +5,10 @@ import Foundation import GRDB +enum BodyIndexingFailureReason: String, Sendable { + case partialFetchUnsupported = "partial_fetch_unsupported" +} + /// TabMail action tags. Raw values are plain action names ("delete", "archive", etc.). /// Action tags are local-only (ADR-IOS-036) — `MessageHeader.actionTag`, /// `MessageAICache.actionTag`, and Device Sync probe state. We no longer @@ -175,6 +179,15 @@ struct MessageHeader: Codable, Equatable, FetchableRecord, PersistableRecord, Id /// Reset by Smart Reindex to give previously-empty messages a fresh chance. var bodyEmptyConfirmed: Bool = false + /// Stable terminal reason explaining why this body could not be indexed. + /// + /// Unlike `bodyEmptyConfirmed`, this does NOT claim the server returned no + /// content, and unlike `bodyComplete`, it does NOT claim the body reached FTS. + /// A non-nil value retires the row from automatic body queues while keeping + /// the missing index entry truthful and visible in sync progress. Smart + /// Reindex clears it so a server/app upgrade gets another attempt. + var bodyIndexingFailureReason: String? + /// How many times a body fetch returned empty (no text, no attachments). /// Used to guard against false empties from partial IMAP responses. /// bodyEmptyConfirmed is only set when emptyFetchCount >= 3. @@ -279,6 +292,7 @@ struct MessageHeader: Codable, Equatable, FetchableRecord, PersistableRecord, Id self.hasAttachments = false self.isReplied = false self.isForwarded = false + self.bodyIndexingFailureReason = nil } } diff --git a/TabMail/Providers/EmailProvider.swift b/TabMail/Providers/EmailProvider.swift index 64eec43d..bdf24140 100644 --- a/TabMail/Providers/EmailProvider.swift +++ b/TabMail/Providers/EmailProvider.swift @@ -192,6 +192,10 @@ struct InlineImage: Sendable { struct FullMessageInfo: Sendable { let header: MessageHeaderInfo + /// UIDVALIDITY from the exact IMAP SELECT that supplied this message's + /// BODYSTRUCTURE and render parts. Nil for providers without mailbox-local + /// UID epochs, or when the server omitted UIDVALIDITY. + let observedUidValidity: Int? let htmlBody: String? let textBody: String? let attachments: [AttachmentInfo] @@ -199,14 +203,21 @@ struct FullMessageInfo: Sendable { /// Pre-fetched ICS calendar data (from pipelined batch fetch). /// When present, renderBody skips the separate fetchAttachment call. let icsData: Data? + /// IMAP MIME sections allowed to contribute render-time attachment data. + /// `nil` for providers whose attachment identifiers are not MIME sections. + /// The background renderer filters its calendar fallback through this set, + /// while `attachments` remains complete BODYSTRUCTURE metadata for taps. + let renderIngredientSections: Set? - init(header: MessageHeaderInfo, htmlBody: String?, textBody: String?, attachments: [AttachmentInfo] = [], inlineImages: [InlineImage] = [], icsData: Data? = nil) { + init(header: MessageHeaderInfo, observedUidValidity: Int? = nil, htmlBody: String?, textBody: String?, attachments: [AttachmentInfo] = [], inlineImages: [InlineImage] = [], icsData: Data? = nil, renderIngredientSections: Set? = nil) { self.header = header + self.observedUidValidity = observedUidValidity self.htmlBody = htmlBody self.textBody = textBody self.attachments = attachments self.inlineImages = inlineImages self.icsData = icsData + self.renderIngredientSections = renderIngredientSections } } @@ -425,8 +436,9 @@ protocol EmailProvider: Sendable { /// Batch fetch full messages for body processing (MessageBody + FTS + rendering). /// IMAP: single connection, one SELECT, bulk BODYSTRUCTURE, per-message body parts. /// Gmail/Exchange: concurrent HTTP fetches (default sequential fallback). - /// Returns successfully fetched messages keyed by message ID. - /// Throws on connection-level errors. Individual message failures are omitted from result. + /// Returns successfully fetched messages keyed by message ID. Connection failures throw; + /// providers may also throw a typed terminal failure identifying the one message whose + /// bounded fetch contract failed. Other individual parse failures are omitted from the result. func fetchMessagesBatch(ids: [String], folder: String) async throws -> [String: FullMessageInfo] } @@ -725,6 +737,15 @@ enum ProviderError: LocalizedError { /// the request boundary so the bug surfaces at its source, not as an opaque /// network error. case syntheticFolderPath(String) + /// A server did not honor the bounded IMAP partial-fetch contract required + /// to index this message without an unbounded response. Deterministic for + /// the current server/app combination: background queues terminalize this + /// message truthfully instead of retrying forever. + case bodyIndexingUnsupported( + messageId: String, + observedUidValidity: Int?, + fetchedRfc822MessageId: String? + ) /// The row's provider address is not corroborated — a move is in flight, so /// `(folderPath, messageId)` may name a DIFFERENT message on the wire. Thrown by /// `AccountManager.fetchAttachment`; see `BodyAddressGate`. TRANSIENT in the DATABASE — it @@ -748,6 +769,8 @@ enum ProviderError: LocalizedError { return "UIDVALIDITY changed for \(folderPath): stored=\(stored) live=\(live)" case .syntheticPlaceholderId(let ids): return "Synthetic placeholder id(s) leaked into provider fetch: \(ids.prefix(3))" case .syntheticFolderPath(let path): return "Synthetic folder path leaked into provider request: \(path)" + case .bodyIndexingUnsupported: + return "This server cannot fetch the message body in bounded pieces, so it was not indexed." // ⚠️ Deliberately names GOING BACK TO THE LIST, not "try again" and not "close it". // An already-open view holds the pre-move `MessageHeader` in memory and `publishMoveFinish` // does not push the re-keyed row into it, so repeated taps re-submit the same stale value diff --git a/TabMail/Providers/IMAPProvider.swift b/TabMail/Providers/IMAPProvider.swift index 8af078ba..4137bf64 100644 --- a/TabMail/Providers/IMAPProvider.swift +++ b/TabMail/Providers/IMAPProvider.swift @@ -3517,7 +3517,9 @@ actor IMAPProvider: EmailProvider, MessageExistenceProbe { // this re-SELECT. It runs on the ACTION connection, which // `withActionConnection` has already SELECTed, so this is the second // SELECT of the pair and the one whose epoch is live at FETCH time. - _ = try await selectMailboxTracked(server, folder: folder) + let selection = try await selectMailboxTracked(server, folder: folder) + let selectedEpoch = selection.uidValidity.value + let observedUidValidity = selectedEpoch == 0 ? nil : Int(selectedEpoch) let results = try nativeUIDSet([id]) guard !results.isEmpty else { @@ -3527,14 +3529,59 @@ actor IMAPProvider: EmailProvider, MessageExistenceProbe { let uids = results.toArray() guard let uid = uids.first else { throw ProviderError.messageNotFound } - let info = try await server.fetchMessageInfo(for: uid) + let info: MessageInfo? + do { + info = try await server.fetchMessageInfo( + for: uid, + options: IMAPFetchMapping.bodyFetchMetadataOptions + ) + } catch { + if IMAPFetchMapping.isResponseBufferOverflow(error) { + throw ProviderError.bodyIndexingUnsupported( + messageId: id, + observedUidValidity: observedUidValidity, + fetchedRfc822MessageId: nil + ) + } + throw error + } guard let info else { throw ProviderError.messageNotFound } - let message = try await server.fetchMessage(from: info) + var parts = info.parts + do { + for index in IMAPFetchMapping.requiredBodyPartIndices(in: parts) { + let section = parts[index].section + let expectedSize = parts[index].size + parts[index].data = try await IMAPFetchMapping.concatenateEncodedPart( + expectedSize: expectedSize + ) { offset, count in + try await server.fetchPart( + section: section, + of: uid, + offset: offset, + count: count + ) + } + } + } catch { + if IMAPFetchMapping.isDeterministicPartialFetchFailure(error) { + throw ProviderError.bodyIndexingUnsupported( + messageId: id, + observedUidValidity: observedUidValidity, + fetchedRfc822MessageId: IMAPFetchMapping.rfc822MessageId(from: info) + ) + } + throw error + } + let message = Message(header: info, parts: parts) // buildFullMessageInfo returns nil when mapMessageInfo can't parse the header // (e.g., date parse failure). Treat as a fetch failure so the caller retries. - guard let full = buildFullMessageInfo(info: info, message: message) else { + guard let full = buildFullMessageInfo( + info: info, + message: message, + observedUidValidity: observedUidValidity + ) else { throw ProviderError.messageNotFound } return full @@ -3543,8 +3590,16 @@ actor IMAPProvider: EmailProvider, MessageExistenceProbe { /// Build FullMessageInfo from BODYSTRUCTURE info + fetched message data. /// Extracted from fetchMessageOnConnection so batch fetch can reuse it. /// Returns nil if the header can't be parsed — caller should treat as fetch failure. - private func buildFullMessageInfo(info: MessageInfo, message: Message) -> FullMessageInfo? { + private func buildFullMessageInfo( + info: MessageInfo, + message: Message, + observedUidValidity: Int? + ) -> FullMessageInfo? { guard let header = mapMessageInfo(info) else { return nil } + let renderIngredientSections = Set( + IMAPFetchMapping.requiredBodyPartIndices(in: info.parts) + .map { info.parts[$0].section.description } + ) // Classify each attachment as top-level vs nested-in-.eml by checking // whether its MIME section is a descendant of any message/rfc822 section. @@ -3576,18 +3631,20 @@ actor IMAPProvider: EmailProvider, MessageExistenceProbe { filename: filename, contentType: part.contentType, section: part.section.description, - size: part.data?.count ?? 0, + size: part.size ?? part.data?.count ?? 0, encoding: part.encoding, parentEmlSection: parentEml ) } - // Surface attachments nested INSIDE file-uploaded `.eml` parts. + // Surface attachments nested INSIDE file-uploaded `.eml` parts when + // parent bytes happen to be present (for example, an on-demand path). // Server-parsed `message/rfc822` parts already have their children // visible at the top level (BODYSTRUCTURE exposes them at numeric // sub-sections like `2.1`, and the block above catches them). - // File-uploaded `.eml`s are opaque blobs server-side — the nested - // attachments only exist after we parse the bytes ourselves. + // File-uploaded `.eml`s are opaque blobs server-side — background body + // indexing deliberately does not download their attachment payloads, so + // BODYSTRUCTURE exposes only the parent until it is opened on demand. // // `encoding` on each nested AttachmentInfo is set to the PARENT's // transfer encoding (not the inner attachment's). Tap-time @@ -3614,23 +3671,23 @@ actor IMAPProvider: EmailProvider, MessageExistenceProbe { } } - // Extract CID inline images — data is already fetched by SwiftMail. - // Use decodedData() to handle content transfer encoding (base64, quoted-printable) - // before we re-encode as data: URI in AccountManagerFetch. - let inlineImages: [InlineImage] = message.cids.prefix(SyncConfig.maxInlineImages).compactMap { part in - guard let rawId = part.contentId, let data = part.decodedData() else { return nil } - // Strip angle brackets + whitespace: "< image001@host >" → "image001@host" - let contentId = rawId.trimmingCharacters(in: .whitespacesAndNewlines) - .trimmingCharacters(in: CharacterSet(charactersIn: "<>")) - .trimmingCharacters(in: .whitespacesAndNewlines) - guard !contentId.isEmpty else { return nil } - return InlineImage(contentId: contentId, contentType: part.contentType, data: data) + let inlineImages = IMAPFetchMapping.extractInlineImages( + message: message, + maxInlineImages: SyncConfig.maxInlineImages, + eligibleSections: renderIngredientSections + ).map { image in + InlineImage( + contentId: image.contentId, + contentType: image.contentType, + data: image.data + ) } // Extract ICS calendar data from already-fetched parts (avoids re-fetch in renderBody) - let icsData: Data? = message.parts.first(where: { - $0.contentType.lowercased().contains("text/calendar") - })?.decodedData() + let icsData = IMAPFetchMapping.extractICSData( + message: message, + eligibleSections: renderIngredientSections + ) // Log body part structure for debugging embedded .eml rendering. // @@ -3675,31 +3732,24 @@ actor IMAPProvider: EmailProvider, MessageExistenceProbe { return FullMessageInfo( header: header, + observedUidValidity: observedUidValidity, htmlBody: htmlBody, textBody: textBody, attachments: attachments, inlineImages: inlineImages, - icsData: icsData + icsData: icsData, + renderIngredientSections: renderIngredientSections ) } // MARK: - Batch Full Message Fetch - /// Batch fetch full messages on a single connection using pipelined part fetches. - /// Flow: one SELECT → one bulk BODYSTRUCTURE → pipelined FETCH for all parts across all messages. - /// Avoids redundant per-message BODYSTRUCTURE re-fetch that `fetchMessage(from:)` does internally. - /// ⚠️ CORRECTED 2026-08-05: this line previously read "PayloadTooLarge messages - /// are marked bodyEmptyConfirmed and omitted from result." That has not been - /// true on this path for some time and the identical stale line is present at - /// the release base `07a4bb703` too, so it is pre-existing rather than a - /// regression. **A `PayloadTooLargeError` is THROWN, not absorbed**: it - /// contaminates the NIO connection (unfulfilled promises crash on dealloc), so - /// the batch is failed and the connection released as unhealthy rather than - /// retried here. Deleting the `bodyEmptyConfirmed = 1` write was the CORRECT - /// change — an oversized body is the opposite of "content confirmed gone", and - /// marking it would violate the Data Integrity rule against marking unfetched - /// content as fetched. The message stays retryable. - /// Throws on connection-level errors (caller should retry the batch). + /// Batch fetch renderable message content on one folder connection. + /// Flow: one SELECT → one bulk BODYSTRUCTURE → bounded partial FETCH commands + /// for text/calendar/CID parts. Normal attachments remain metadata-only and + /// are fetched on demand. A server that ignores or malforms a partial range is + /// surfaced as a typed per-message terminal failure; connection/transient + /// errors still fail the batch for retry. func fetchMessagesBatch(ids: [String], folder: String) async throws -> [String: FullMessageInfo] { // Defensive guard — see `EmailProvider.fetchMessagesBatch` extension default // for the rationale. The implicit `UInt32(id)` filter below would already @@ -3722,7 +3772,12 @@ actor IMAPProvider: EmailProvider, MessageExistenceProbe { if DebugModeManager.isLoggingEnabled() { print("[IMAP] fetchMessagesBatch START: \(uidPairs.count) UIDs in \(folder)") } - return try await withFolderConnection(folder: folder) { server in + var partialFetchMessageId: String? + var metadataFetchMessageId: String? + var partialFetchObservedUidValidity: Int? + var partialFetchRfc822MessageId: String? + do { + return try await withFolderConnection(folder: folder) { server in // 1. SELECT (re-selects on pinned connection — fast, refreshes state) let tSelect = CFAbsoluteTimeGetCurrent() // T5.3 PORT — `v2final:…:IMAPProvider.fetchMessagesBatch` tracks this @@ -3730,7 +3785,9 @@ actor IMAPProvider: EmailProvider, MessageExistenceProbe { // hot path and runs concurrently with the backfill walk on the SAME // folder path, so it is one of the SELECTs most likely to be the // first to see a turnover. - _ = try await selectMailboxTracked(server, folder: folder) + let selection = try await selectMailboxTracked(server, folder: folder) + let selectedEpoch = selection.uidValidity.value + partialFetchObservedUidValidity = selectedEpoch == 0 ? nil : Int(selectedEpoch) let selectMs = Int((CFAbsoluteTimeGetCurrent() - tSelect) * 1000) if DebugModeManager.isLoggingEnabled() { print("[IMAP] fetchMessagesBatch SELECT: \(selectMs)ms") } @@ -3738,7 +3795,12 @@ actor IMAPProvider: EmailProvider, MessageExistenceProbe { let tStruct = CFAbsoluteTimeGetCurrent() var uidSet = UIDSet() for (_, uid) in uidPairs { uidSet.insert(UID(uid)) } - let infos = try await server.fetchMessageInfosBulk(using: uidSet) + metadataFetchMessageId = uidPairs.count == 1 ? uidPairs[0].id : nil + let infos = try await server.fetchMessageInfosBulk( + using: uidSet, + options: IMAPFetchMapping.bodyFetchMetadataOptions + ) + metadataFetchMessageId = nil let structMs = Int((CFAbsoluteTimeGetCurrent() - tStruct) * 1000) if DebugModeManager.isLoggingEnabled() { print("[IMAP] fetchMessagesBatch BODYSTRUCTURE: \(infos.count)/\(uidPairs.count) returned in \(structMs)ms") } @@ -3752,72 +3814,72 @@ actor IMAPProvider: EmailProvider, MessageExistenceProbe { } } - // 3. Collect ALL parts from ALL messages for pipelined fetch. - // We already have BODYSTRUCTURE from step 2 — no need to re-fetch it - // (fetchMessage(from:) internally calls fetchStructure again, which is wasteful - // and causes NIO buffer accumulation with 200+ redundant IMAP commands). - var partRequests: [(uid: UID, section: Section)] = [] + // 3. Fetch only render-required parts in bounded encoded-byte chunks. + // BODYSTRUCTURE already supplies attachment metadata, so normal file + // attachment payloads are intentionally absent from this background path. + // Each part is transfer-decoded only after all encoded chunks are joined. + let tParts = CFAbsoluteTimeGetCurrent() var partsByUID: [UInt32: [MessagePart]] = [:] + var fetchedSectionsByUID: [UInt32: Set] = [:] + var totalParts = 0 for (uidValue, entry) in infoByUID { let uid = UID(uidValue) - partsByUID[uidValue] = entry.info.parts - for part in entry.info.parts { - partRequests.append((uid: uid, section: part.section)) + var parts = entry.info.parts + var fetchedSections: Set = [] + for index in IMAPFetchMapping.requiredBodyPartIndices(in: parts) { + totalParts += 1 + partialFetchMessageId = entry.id + partialFetchRfc822MessageId = IMAPFetchMapping.rfc822MessageId( + from: entry.info + ) + let section = parts[index].section + let expectedSize = parts[index].size + parts[index].data = try await IMAPFetchMapping.concatenateEncodedPart( + expectedSize: expectedSize + ) { offset, count in + try await server.fetchPart( + section: section, + of: uid, + offset: offset, + count: count + ) + } + fetchedSections.insert(section.description) } - } - - let totalParts = partRequests.count - if DebugModeManager.isLoggingEnabled() { print("[IMAP] fetchMessagesBatch: \(totalParts) parts to fetch across \(infoByUID.count) messages") } - - // 4. Pipelined fetch — all parts in one burst. - // PayloadTooLarge contaminates the NIO connection (unfulfilled promises crash - // on dealloc), so we do NOT retry here. Instead, throw to the queue which - // handles halving with a fresh connection on each retry. - let tParts = CFAbsoluteTimeGetCurrent() - let pipelinedResults: [UID: [(section: Section, data: Data)]] - if !partRequests.isEmpty { - pipelinedResults = try await server.fetchPartsPipelined(parts: partRequests) - } else { - pipelinedResults = [:] + partialFetchMessageId = nil + partialFetchRfc822MessageId = nil + partsByUID[uidValue] = parts + fetchedSectionsByUID[uidValue] = fetchedSections } let partsMs = Int((CFAbsoluteTimeGetCurrent() - tParts) * 1000) - if DebugModeManager.isLoggingEnabled() { print("[IMAP] fetchMessagesBatch PARTS PIPELINED: \(pipelinedResults.count) UIDs returned in \(partsMs)ms") } + if DebugModeManager.isLoggingEnabled() { print("[IMAP] fetchMessagesBatch PARTS CHUNKED: \(totalParts) render parts across \(infoByUID.count) messages in \(partsMs)ms") } - // 5. Assemble Message objects from BODYSTRUCTURE + fetched part data + // 4. Assemble Message objects from BODYSTRUCTURE + fetched part data. var results: [String: FullMessageInfo] = [:] var fetchedCount = 0 var failedCount = 0 for (uidValue, entry) in infoByUID { - let uid = UID(uidValue) - guard var parts = partsByUID[uidValue] else { continue } - - // Populate part data from pipelined results - let fetchedParts = pipelinedResults[uid] ?? [] - var fetchedBySection: [String: Data] = [:] - for (section, data) in fetchedParts { - fetchedBySection[section.description] = data - } - - for i in 0.. Data { + func fetchAttachment( + messageId: String, + folder: String, + section: String, + encoding: String?, + expectedObservedUidValidity: Int?, + expectedRfc822MessageId: String? + ) async throws -> Data { // Compound path: attachment nested inside a file-uploaded `.eml`. // Re-fetch parent `.eml` bytes, parse, return the nth nested payload. // One extra IMAP fetch per tap — the parse happens in-process. if let nested = EmlParsing.parseNestedSection(section) { let parentBytes = try await fetchAttachment( - messageId: messageId, folder: folder, section: nested.parent, encoding: encoding + messageId: messageId, + folder: folder, + section: nested.parent, + encoding: encoding, + expectedObservedUidValidity: expectedObservedUidValidity, + expectedRfc822MessageId: expectedRfc822MessageId ) guard let bytes = EmlParsing.nestedBytes(rawBytes: parentBytes, index: nested.index) else { throw ProviderError.messageNotFound @@ -5238,19 +5331,91 @@ actor IMAPProvider: EmailProvider, MessageExistenceProbe { return try await withActionConnection(folder: folder) { server in // T5.3 PORT — `v2final:…:IMAPProvider.fetchAttachment` tracks this // re-SELECT on the action connection. - _ = try await self.selectMailboxTracked(server, folder: folder) + let selection = try await self.selectMailboxTracked(server, folder: folder) let results = try self.nativeUIDSet([messageId]) guard let uid = results.toArray().first else { throw ProviderError.messageNotFound } let mimeSection = Section(section) - let rawData = try await server.fetchPart(section: mimeSection, of: uid) + guard let info = try await server.fetchMessageInfo( + for: uid, + options: IMAPFetchMapping.bodyFetchMetadataOptions + ) else { + throw ProviderError.messageNotFound + } + try Self.requireAttachmentFetchIdentity( + messageId: messageId, + folder: folder, + expectedObservedUidValidity: expectedObservedUidValidity, + liveUidValidity: selection.uidValidity.value, + expectedRfc822MessageId: expectedRfc822MessageId, + fetchedRfc822MessageId: IMAPFetchMapping.rfc822MessageId(from: info) + ) + guard let metadata = info.parts.first(where: { $0.section == mimeSection }) else { + throw ProviderError.messageNotFound + } + let rawData = try await IMAPFetchMapping.concatenateEncodedPart( + expectedSize: metadata.size + ) { offset, count in + try await server.fetchPart( + section: mimeSection, + of: uid, + offset: offset, + count: count + ) + } - let part = MessagePart(section: mimeSection, contentType: "", encoding: encoding) + let part = MessagePart( + section: mimeSection, + contentType: metadata.contentType, + encoding: encoding ?? metadata.encoding + ) return rawData.decoded(for: part) } } + /// Bind an attachment read to the row that initiated it. A mailbox-local + /// UID is safe only while its UIDVALIDITY agrees; when either side lacks an + /// epoch, matching RFC Message-ID is the fallback positive identity proof. + private static func requireAttachmentFetchIdentity( + messageId: String, + folder: String, + expectedObservedUidValidity: Int?, + liveUidValidity: UInt32, + expectedRfc822MessageId: String?, + fetchedRfc822MessageId: String? + ) throws { + let expectedEpoch = expectedObservedUidValidity.flatMap(UInt32.init(exactly:)) + .flatMap { $0 == 0 ? nil : $0 } + let liveEpoch = liveUidValidity == 0 ? nil : liveUidValidity + + if let expectedEpoch, let liveEpoch { + guard expectedEpoch == liveEpoch else { + throw ProviderError.uidValidityChanged( + folderPath: folder, + stored: expectedEpoch, + live: liveEpoch + ) + } + guard !BodyAddressGate.identityContradicts( + stored: expectedRfc822MessageId, + fetched: fetchedRfc822MessageId + ) else { + throw ProviderError.actionIdentityResolutionFailed(messageId) + } + return + } + + guard let expectedRfc822MessageId, + let fetchedRfc822MessageId, + !expectedRfc822MessageId.isEmpty, + !fetchedRfc822MessageId.isEmpty, + EmailFilter.normalizeMessageId(expectedRfc822MessageId) + == EmailFilter.normalizeMessageId(fetchedRfc822MessageId) else { + throw ProviderError.actionIdentityResolutionFailed(messageId) + } + } + func send(draft: DraftMessage) async throws { try await withTimeout(seconds: SyncConfig.smtpSendTimeoutSeconds) { if DebugModeManager.isLoggingEnabled() { print("[SMTP] Sending via \(self.smtpHost):\(self.smtpPort) from=\(self.senderEmail) to=\(draft.to) attachments=\(draft.attachments.count)") } @@ -5949,7 +6114,16 @@ actor IMAPProvider: EmailProvider, MessageExistenceProbe { for part in textParts { do { - let data = try await server.fetchPart(section: part.section, of: uid) + let data = try await IMAPFetchMapping.concatenateEncodedPart( + expectedSize: part.size + ) { offset, count in + try await server.fetchPart( + section: part.section, + of: uid, + offset: offset, + count: count + ) + } var populated = part populated.data = data if part.contentType.lowercased().hasPrefix("text/plain"), textBody == nil { diff --git a/TabMail/Services/Account/AccountManager.swift b/TabMail/Services/Account/AccountManager.swift index b6c856f9..a11213aa 100644 --- a/TabMail/Services/Account/AccountManager.swift +++ b/TabMail/Services/Account/AccountManager.swift @@ -35,6 +35,10 @@ struct BackfillProgress { /// counts a different population than what we store (see `isFullyComplete`). var pendingBodyCount: Int = 0 + /// Bodies retired from automatic indexing with a truthful terminal reason. + /// These rows are not indexed and are not counted as pending work. + var unindexedBodyCount: Int = 0 + // EMA rate tracking — messages per second. var lastIndexedCount: Int = 0 var lastRateUpdate: Date = .distantPast @@ -46,6 +50,7 @@ struct BackfillProgress { /// UID walk progress is more accurate during backfill because UIDNEXT gives /// a known total scope, unlike FTS where the denominator keeps growing. var fractionComplete: Double { + if isFullyComplete { return 1.0 } if !headersDone && uidTotal > 0 { return min(1.0, Double(uidWalked) / Double(uidTotal)) } @@ -63,7 +68,8 @@ struct BackfillProgress { /// it's the dedup'd `messagesTotal` vs. our per-label rows. It can permanently /// exceed what we can ever store, so an `ftsIndexed >= totalEmails` gate would /// never be satisfiable and "Sync Complete" would never fire. `pendingBodyCount` - /// is local and self-terminating (empty/404/oversized bodies confirm-empty), + /// is local and self-terminating (empty/404 bodies confirm-empty; deterministic + /// protocol failures enter the separate terminal-unindexed state), /// so it reaches 0 once the body queues have nothing fetchable left. var isFullyComplete: Bool { headersDone && totalEmails > 0 && pendingBodyCount == 0 @@ -95,6 +101,22 @@ struct BackfillProgress { } } +enum BodyIndexingProgressText { + static func completion(unindexedCount: Int) -> String { + guard unindexedCount > 0 else { return "Sync complete" } + let noun = unindexedCount == 1 ? "message" : "messages" + return "Sync complete with \(unindexedCount.formatted()) \(noun) not indexed" + } + + /// Shared presentation decision for aggregate and per-account sync views. + /// Returning nil keeps in-progress/index-count branches separate while + /// ensuring every completed-with-omissions surface uses the exact wording. + static func terminalCompletion(isComplete: Bool, unindexedCount: Int) -> String? { + guard isComplete, unindexedCount > 0 else { return nil } + return completion(unindexedCount: unindexedCount) + } +} + /// Deduplicates concurrent OAuth refresh calls for a single account. /// Prevents race where mail + calendar providers both get 401 and refresh the same token. /// Microsoft rotates refresh tokens on use — second caller with the old token would fail. @@ -1163,7 +1185,8 @@ actor AccountManager { func updateBackfillProgress(accountId: String, email: String, headersDone: Bool, isPaused: Bool, totalEmails: Int = 0, ftsIndexed: Int = 0, uidTotal: Int = 0, uidWalked: Int = 0, - pendingBodyCount: Int = 0) { + pendingBodyCount: Int = 0, + unindexedBodyCount: Int = 0) { if var existing = _backfillBacking[accountId] { existing.headersDone = headersDone existing.isPaused = isPaused @@ -1172,6 +1195,7 @@ actor AccountManager { existing.uidTotal = uidTotal existing.uidWalked = uidWalked existing.pendingBodyCount = pendingBodyCount + existing.unindexedBodyCount = unindexedBodyCount existing.updateRate() _backfillBacking[accountId] = existing } else { @@ -1183,6 +1207,7 @@ actor AccountManager { progress.uidTotal = uidTotal progress.uidWalked = uidWalked progress.pendingBodyCount = pendingBodyCount + progress.unindexedBodyCount = unindexedBodyCount progress.lastIndexedCount = ftsIndexed progress.lastRateUpdate = Date() _backfillBacking[accountId] = progress diff --git a/TabMail/Services/Account/AccountManagerFetch.swift b/TabMail/Services/Account/AccountManagerFetch.swift index 3a36e3a2..f7988276 100644 --- a/TabMail/Services/Account/AccountManagerFetch.swift +++ b/TabMail/Services/Account/AccountManagerFetch.swift @@ -106,6 +106,14 @@ extension AccountManager { let hasBody = (try? await dbPool.read { db in try MessageBody.fetchOne(db, key: message.id) != nil }) ?? false guard replaceExistingBody || !hasBody else { return } + if await bodyIndexingFailureReason(forHeaderId: message.id) != nil { + throw ProviderError.bodyIndexingUnsupported( + messageId: message.messageId, + observedUidValidity: nil, + fetchedRfc822MessageId: nil + ) + } + // Address not corroborated: this UID names a DIFFERENT message on the wire right now, // and `BodyFetchProcessor.process` would refuse the write anyway. Skip the round trip. // @@ -164,6 +172,13 @@ extension AccountManager { userInfo: [NSLocalizedDescriptionKey: "This message is too large to display."]) ) case .retry: + if await bodyIndexingFailureReason(forHeaderId: message.id) != nil { + throw ProviderError.bodyIndexingUnsupported( + messageId: message.messageId, + observedUidValidity: nil, + fetchedRfc822MessageId: nil + ) + } throw ProviderError.networkError( underlying: NSError(domain: "TabMail", code: -2, userInfo: [NSLocalizedDescriptionKey: "Failed to load message. Please try again."]) @@ -171,6 +186,20 @@ extension AccountManager { } } + /// Durable terminal body state used by the detail loader and its poll. + /// Read by primary key each time so a row terminalized while already open + /// stops the next poll tick without another network request. + func bodyIndexingFailureReason(forHeaderId headerId: String) async -> BodyIndexingFailureReason? { + let raw = try? await dbPool.pool.read { db in + try String.fetchOne( + db, + sql: "SELECT bodyIndexingFailureReason FROM messageHeader WHERE id = ?", + arguments: [headerId] + ) + } + return raw.flatMap(BodyIndexingFailureReason.init(rawValue:)) + } + /// Download a single attachment's data. /// On connection error, reconnects the provider and retries once (handles stale IMAP after device sleep). func fetchAttachment(for message: MessageHeader, section: String, encoding: String?) async throws -> Data { @@ -205,7 +234,14 @@ extension AccountManager { do { return try await queue.execute(priority: .userAction) { if let imapProvider = provider as? IMAPProvider { - return try await imapProvider.fetchAttachment(messageId: message.messageId, folder: message.folderPath, section: section, encoding: encoding) + return try await imapProvider.fetchAttachment( + messageId: message.messageId, + folder: message.folderPath, + section: section, + encoding: encoding, + expectedObservedUidValidity: message.observedUidValidity, + expectedRfc822MessageId: message.rfc822MessageId + ) } else if let gmailProvider = provider as? GmailProvider { return try await gmailProvider.fetchAttachment(messageId: message.messageId, attachmentId: section) } else if let exchangeProvider = provider as? ExchangeProvider { diff --git a/TabMail/Services/AppDatabase.swift b/TabMail/Services/AppDatabase.swift index 8a26a78c..dc803266 100644 --- a/TabMail/Services/AppDatabase.swift +++ b/TabMail/Services/AppDatabase.swift @@ -1999,7 +1999,7 @@ final class AppDatabase: Sendable { } } - // ── FOREIGN-KEY CHECK MODE FOR THE v68…v87 RANGE ───────────────────── + // ── FOREIGN-KEY CHECK MODE FOR THE v68…v88 RANGE ───────────────────── // // `registerTimedMigration`'s DEFAULT stays `.deferred` and is NOT // changed. v1…v67 have never been adjudicated for `.immediate` safety, @@ -2071,6 +2071,8 @@ final class AppDatabase: Sendable { // • v87 — drops only v85/v86's direct-AI triggers, sparse index and two // non-key marker columns. It neither reads nor rewrites an FK-bearing // value; existing derived-work markers are intentionally discarded. + // • v88 — adds one nullable non-key messageHeader column and a partial + // index. Neither statement writes a parent or child key. // // • v82 — `DROP`/`CREATE` of `userLabel` + `messageUserLabel`. FK-clean // per statement, verified statement by statement in that migration's own @@ -2081,16 +2083,16 @@ final class AppDatabase: Sendable { // the body safe. // // ⚑ AMENDED 2026-08-06, RANGE RE-DERIVED AT R17-6 — **EVERY MIGRATION IN - // v68…v87 NOW RUNS `.immediate`, so this range runs ZERO whole-database + // v68…v88 NOW RUNS `.immediate`, so this range runs ZERO whole-database // foreign-key checks.** The range is an OPEN interval that moves with the // top of the chain, so it is re-derived rather than restated (`MIS-031` — a // sentence that enumerates is a cache, and this one had gone stale at `v84` // in five places at once). Comments excluded so this paragraph cannot // satisfy its own predicate (`MIS-033`, `IOS-DOC-002`): // rg -c --pcre2 '^(?!\s*(///|//)).*foreignKeyChecks: \.immediate' \ - // TabMail/Services/AppDatabase.swift → 20 + // TabMail/Services/AppDatabase.swift → 21 // rg -o '"v([0-9]+)_[A-Za-z0-9_]+"' -r '$1' \ - // TabMail/Services/AppDatabase.swift | sort -n -u | awk '$1>=68' | wc -l → 20 + // TabMail/Services/AppDatabase.swift | sort -n -u | awk '$1>=68' | wc -l → 21 // Equal counts are the invariant: every migration from v68 to the top runs // `.immediate`, and none below v68 does. The sentence // that stood here said *"`v71` and `v82` stay `.deferred`, each for a @@ -3013,7 +3015,12 @@ final class AppDatabase: Sendable { // `.deferred`, **66** default ⇒ **67 of 87** end with the // whole-database check and 20 do not. The 67 is unchanged // because v87 joined the immediate side. - // ⚠️ THIS MIGRATION IS ONE OF THE 20: reason (b) below is now a + // Re-derived after v88: **88** registered, **21** explicit + // `.immediate` (v68…v88, contiguous), **1** explicit + // `.deferred`, **66** default ⇒ **67 of 88** end with the + // whole-database check and 21 do not. The 67 is unchanged + // because v88 joined the immediate side. + // ⚠️ THIS MIGRATION IS ONE OF THE 21: reason (b) below is now a // statement about migrations that // ran on shipped devices long ago, not about this chain. // Confirmed against GRDB's own @@ -3682,6 +3689,34 @@ final class AppDatabase: Sendable { try db.execute(sql: "ALTER TABLE messageHeader DROP COLUMN aiDirectPending") try db.execute(sql: "ALTER TABLE messageAICache DROP COLUMN aiDirectPending") } + + // v88 — record terminal body-indexing failures without misclassifying + // real server content as empty or indexed. + // + // The partial-fetch path can prove that a server ignored or malformed a + // requested BODY.PEEK range. Retrying that deterministic protocol failure + // forever leaves Fast Sync permanently active. This nullable reason is a + // separate truth state: bodyComplete remains false, bodyEmptyConfirmed + // remains false, and automatic queues exclude the row until Smart Reindex + // explicitly clears the reason. + migrator.registerTimedMigration( + "v88_addBodyIndexingFailureReason", foreignKeyChecks: .immediate + ) { db in + try db.alter(table: "messageHeader") { t in + t.add(column: "bodyIndexingFailureReason", .text) + } + // Match the queue predicate exactly. At steady state this index is + // empty; on upgrade it contains only genuinely pending rows rather + // than duplicating the entire large messageHeader table. + try db.execute(sql: """ + CREATE INDEX IF NOT EXISTS messageHeader_bodyIndexingQueue + ON messageHeader(isInInbox, date) + WHERE headerComplete = 1 + AND bodyComplete = 0 + AND bodyEmptyConfirmed = 0 + AND bodyIndexingFailureReason IS NULL + """) + } } /// PORT — v2final `AppDatabase.seedDraftLastTouchedSeq`. Extracted to a static diff --git a/TabMail/Services/NSEDataBridge.swift b/TabMail/Services/NSEDataBridge.swift index 9636453d..dcfd434c 100644 --- a/TabMail/Services/NSEDataBridge.swift +++ b/TabMail/Services/NSEDataBridge.swift @@ -3280,11 +3280,7 @@ enum NSEDataBridge { let confirmedIds: [String] = confirmedItems.map(\.item.headerId) do { try await AppDatabase.dbPool.write { db in - let placeholders = confirmedIds.map { _ in "?" }.joined(separator: ",") - try db.execute( - sql: "UPDATE messageHeader SET bodyComplete = 1 WHERE id IN (\(placeholders))", - arguments: StatementArguments(confirmedIds) - ) + try markConfirmedBodiesComplete(confirmedIds, in: db) } } catch { print("[NSEDataBridge] FTS batch: bodyComplete update failed: \(error)") @@ -3355,6 +3351,23 @@ enum NSEDataBridge { } } + /// Commit the successful NSE body-indexing outcome as one state transition. + /// A foreground Smart Reindex may have left a durable terminal reason on + /// the row before a later NSE fetch succeeds; success must clear that reason + /// at the same time it records `bodyComplete`. + static func markConfirmedBodiesComplete(_ headerIds: [String], in db: Database) throws { + guard !headerIds.isEmpty else { return } + let placeholders = headerIds.map { _ in "?" }.joined(separator: ",") + try db.execute( + sql: """ + UPDATE messageHeader + SET bodyComplete = 1, bodyIndexingFailureReason = NULL + WHERE id IN (\(placeholders)) + """, + arguments: StatementArguments(headerIds) + ) + } + /// When NSE delivered an active (reminder) notification, mark the same /// reminder hash in ReachedOutStore so ProactiveNotifyService's dedup path /// won't re-deliver a second notification for the same reminder. diff --git a/TabMail/Services/StuckMessageDiagnostics.swift b/TabMail/Services/StuckMessageDiagnostics.swift index fbb234e0..aab82a74 100644 --- a/TabMail/Services/StuckMessageDiagnostics.swift +++ b/TabMail/Services/StuckMessageDiagnostics.swift @@ -29,6 +29,15 @@ enum StuckMessageDiagnostics { /// Bounded per-class sample size — display-only, full data stays in the DB. private static let sampleLimit = 40 + struct BodyStatusCounts: Equatable, Sendable { + let lockedEmpty: Int + let failing: Int + let pending: Int + let terminalUnindexed: Int + + var runnable: Int { failing + pending } + } + private struct Row: Sendable { let id: String let folderId: String @@ -45,6 +54,7 @@ enum StuckMessageDiagnostics { let folderExists: Bool let hasHealthySibling: Bool let emptyFetchCount: Int + let bodyIndexingFailureReason: String? } static func run() async { @@ -68,16 +78,14 @@ enum StuckMessageDiagnostics { let pkMismatchBodyless = await count(pool, "m.id <> m.accountId || ':' || m.folderPath || ':' || m.messageId AND m.bodyComplete = 0") // Bodyless breakdown. let bodyless = await count(pool, "m.bodyComplete = 0") - let bodylessLocked = await count(pool, "m.bodyComplete = 0 AND m.bodyEmptyConfirmed = 1") - let bodylessFailing = await count(pool, "m.bodyComplete = 0 AND m.bodyEmptyConfirmed = 0 AND m.emptyFetchCount > 0") - let bodylessPending = await count(pool, "m.bodyComplete = 0 AND m.bodyEmptyConfirmed = 0 AND m.emptyFetchCount = 0") + let bodyStatuses = await bodyStatusCounts(in: pool) // Missing rfc822 among the not-browsable set → UID-remap can never recover. let notBrowsableNoRfc = await count(pool, "f.id IS NULL AND (m.rfc822MessageId IS NULL OR m.rfc822MessageId = '')") BackgroundSyncLogger.logStuckDiag("total messageHeader rows: \(total)") BackgroundSyncLogger.logStuckDiag("NOT-browsable (folderId matches no folder): \(notBrowsable) [empty=\(notBrowsableEmpty), orphan=\(notBrowsableOrphan), bodyless=\(notBrowsableBodyless), missing-rfc822=\(notBrowsableNoRfc)]") BackgroundSyncLogger.logStuckDiag("PK/folder mismatch (optimistic-move remnant): \(pkMismatch) [bodyless=\(pkMismatchBodyless)]") - BackgroundSyncLogger.logStuckDiag("bodyless (bodyComplete=0): \(bodyless) [lockedEmpty=\(bodylessLocked), failing=\(bodylessFailing), pending=\(bodylessPending)]") + BackgroundSyncLogger.logStuckDiag("bodyless (bodyComplete=0): \(bodyless) [lockedEmpty=\(bodyStatuses.lockedEmpty), failing=\(bodyStatuses.failing), pending=\(bodyStatuses.pending), terminalUnindexed=\(bodyStatuses.terminalUnindexed)]") // --- Per-provider breakdown of not-browsable ----------------------- let byProvider = await group(pool, @@ -108,7 +116,8 @@ enum StuckMessageDiagnostics { // --- Interpretation hint (ranked by MAGNITUDE, not check order) ---- let buckets: [(Int, String)] = [ - (bodyless, "bodyless body-text backlog (\(bodyless)) — searchable by header, no snippet until body is fetched/indexed. Cured by re-walk (Smart Reindex re-fetches); NOT corruption. Heaviest in custom folders (fetched last)."), + (bodyStatuses.runnable, "runnable body-text backlog (\(bodyStatuses.runnable)) — searchable by header, no snippet until body is fetched/indexed. Smart Reindex re-walks it; terminal-unindexed rows are counted separately."), + (bodyStatuses.terminalUnindexed, "terminal-unindexed bodies (\(bodyStatuses.terminalUnindexed)) — sync is complete, but these servers could not provide bounded body ranges. Smart Reindex explicitly retries them."), (pkMismatch, "PK/folder mismatch (\(pkMismatch)) — optimistic move; benign where messageId is move-stable (Gmail), a stale-UID hazard on IMAP."), (notBrowsable, "NOT-browsable orphaned folderId (\(notBrowsable)) — searchable but in no browsable folder; needs orphan-cleanup/relocate. Smart Reindex does not touch folderId."), ] @@ -126,6 +135,30 @@ enum StuckMessageDiagnostics { }) ?? 0 } + static func bodyStatusCounts(in pool: DatabasePool) async -> BodyStatusCounts { + (try? await pool.read { db -> BodyStatusCounts in + let row = try GRDB.Row.fetchOne(db, sql: """ + SELECT + SUM(CASE WHEN bodyComplete = 0 AND bodyEmptyConfirmed = 1 THEN 1 ELSE 0 END) AS lockedEmpty, + SUM(CASE WHEN bodyComplete = 0 AND bodyEmptyConfirmed = 0 + AND bodyIndexingFailureReason IS NULL + AND emptyFetchCount > 0 THEN 1 ELSE 0 END) AS failing, + SUM(CASE WHEN bodyComplete = 0 AND bodyEmptyConfirmed = 0 + AND bodyIndexingFailureReason IS NULL + AND emptyFetchCount = 0 THEN 1 ELSE 0 END) AS pending, + SUM(CASE WHEN bodyComplete = 0 AND bodyIndexingFailureReason IS NOT NULL + THEN 1 ELSE 0 END) AS terminalUnindexed + FROM messageHeader + """) + return BodyStatusCounts( + lockedEmpty: (row?["lockedEmpty"] as Int?) ?? 0, + failing: (row?["failing"] as Int?) ?? 0, + pending: (row?["pending"] as Int?) ?? 0, + terminalUnindexed: (row?["terminalUnindexed"] as Int?) ?? 0 + ) + }) ?? BodyStatusCounts(lockedEmpty: 0, failing: 0, pending: 0, terminalUnindexed: 0) + } + private static func group(_ pool: DatabasePool, sql: String) async -> [(String, Int)] { (try? await pool.read { db -> [(String, Int)] in try GRDB.Row.fetchAll(db, sql: sql).map { r in @@ -208,7 +241,9 @@ enum StuckMessageDiagnostics { CAST(m.date AS TEXT) AS dateText, (m.rfc822MessageId IS NOT NULL AND m.rfc822MessageId <> '') AS hasRfc, m.bodyComplete AS bodyComplete, m.bodyEmptyConfirmed AS bodyEmpty, - m.emptyFetchCount AS emptyCnt, m.isInInbox AS isInbox, + m.emptyFetchCount AS emptyCnt, + m.bodyIndexingFailureReason AS bodyFailureReason, + m.isInInbox AS isInbox, (f.id IS NOT NULL) AS folderExists, COALESCE(f.role, '-') AS folderRole, COALESCE(a.provider, '?') AS provider, EXISTS(SELECT 1 FROM messageHeader s JOIN folder sf ON s.folderId = sf.id @@ -238,7 +273,8 @@ enum StuckMessageDiagnostics { isInInbox: ((r["isInbox"] as Int?) ?? 0) != 0, folderExists: ((r["folderExists"] as Int?) ?? 0) != 0, hasHealthySibling: ((r["sibling"] as Int?) ?? 0) != 0, - emptyFetchCount: (r["emptyCnt"] as Int?) ?? 0 + emptyFetchCount: (r["emptyCnt"] as Int?) ?? 0, + bodyIndexingFailureReason: r["bodyFailureReason"] as String? ) } }) ?? [] @@ -263,7 +299,7 @@ enum StuckMessageDiagnostics { let inFTS = missingFromFTS.contains(ContentKey(rawValue: r.id)) ? "n" : "Y" // Display-only: id/messageId/subject are abbreviated, full data stays in DB. BackgroundSyncLogger.logStuckDiag( - " id=\(r.id) prov=\(r.provider) folderId=\(r.folderId.isEmpty ? "''" : r.folderId) path=\(r.folderPath) msgId=\(r.messageId) rfc822=\(r.hasRfc822 ? "Y" : "n") body=\(r.bodyComplete ? "Y" : "n") emptyConf=\(r.bodyEmptyConfirmed ? "Y" : "n") emptyCnt=\(r.emptyFetchCount) inbox=\(r.isInInbox ? "Y" : "n") folderExists=\(r.folderExists ? "Y" : "n") role=\(r.folderRole) sibling=\(r.hasHealthySibling ? "Y" : "n") inFTS=\(inFTS) date=\(r.dateStr) subj=\"\(r.subject)\"" + " id=\(r.id) prov=\(r.provider) folderId=\(r.folderId.isEmpty ? "''" : r.folderId) path=\(r.folderPath) msgId=\(r.messageId) rfc822=\(r.hasRfc822 ? "Y" : "n") body=\(r.bodyComplete ? "Y" : "n") emptyConf=\(r.bodyEmptyConfirmed ? "Y" : "n") emptyCnt=\(r.emptyFetchCount) bodyFailure=\(r.bodyIndexingFailureReason ?? "none") inbox=\(r.isInInbox ? "Y" : "n") folderExists=\(r.folderExists ? "Y" : "n") role=\(r.folderRole) sibling=\(r.hasHealthySibling ? "Y" : "n") inFTS=\(inFTS) date=\(r.dateStr) subj=\"\(r.subject)\"" ) } } diff --git a/TabMail/Services/Sync/ActiveBodyQueue.swift b/TabMail/Services/Sync/ActiveBodyQueue.swift index 890a14f0..b924e6ef 100644 --- a/TabMail/Services/Sync/ActiveBodyQueue.swift +++ b/TabMail/Services/Sync/ActiveBodyQueue.swift @@ -339,7 +339,8 @@ actor ActiveBodyQueue { try Row.fetchAll(db, sql: """ SELECT id, accountId, folderPath, messageId, isInInbox FROM messageHeader - WHERE headerComplete = 1 AND bodyComplete = 0 AND bodyEmptyConfirmed = 0 AND isInInbox = 1 + WHERE headerComplete = 1 AND bodyComplete = 0 AND bodyEmptyConfirmed = 0 + AND bodyIndexingFailureReason IS NULL AND isInInbox = 1 ORDER BY date DESC """) .map { row in @@ -615,7 +616,32 @@ actor ActiveBodyQueue { } catch { let desc = "\(error)" - if desc.contains("PayloadTooLargeError") { + if let providerError = error as? ProviderError, + case .bodyIndexingUnsupported( + let messageId, + let observedUidValidity, + let fetchedRfc822MessageId + ) = providerError, + let failedItem = items.first(where: { $0.messageId == messageId }) { + let processorItem = BodyFetchProcessor.Item( + headerId: failedItem.headerId, + accountId: failedItem.accountId, + folderPath: failedItem.folderPath, + messageId: failedItem.messageId, + isInInbox: failedItem.isInInbox + ) + let terminalized = await BodyFetchProcessor.markBodyUnindexed( + item: processorItem, + reason: .partialFetchUnsupported, + observedUidValidity: observedUidValidity, + fetchedRfc822MessageId: fetchedRfc822MessageId + ) + print("[ActiveBody] Partial fetch unsupported — \(terminalized ? "recorded terminal-unindexed state" : "row changed; retrying")") + self.batchItemDone(item: failedItem, shouldRetry: !terminalized) + for item in items where item.headerId != failedItem.headerId { + self.batchItemDone(item: item, shouldRetry: true) + } + } else if desc.contains("PayloadTooLargeError") { // Defer a genuinely single oversized item WITHOUT marking it // empty; a multi-item batch isolates its members so a later // dispatch slices each one singly. The decision keys on THIS @@ -859,7 +885,8 @@ actor ActiveBodyQueue { try Row.fetchAll(db, sql: """ SELECT id, accountId, folderPath, messageId, isInInbox FROM messageHeader - WHERE headerComplete = 1 AND bodyComplete = 0 AND bodyEmptyConfirmed = 0 AND isInInbox = 1 + WHERE headerComplete = 1 AND bodyComplete = 0 AND bodyEmptyConfirmed = 0 + AND bodyIndexingFailureReason IS NULL AND isInInbox = 1 ORDER BY date DESC """) .map { row in diff --git a/TabMail/Services/Sync/BackfillBodyQueue.swift b/TabMail/Services/Sync/BackfillBodyQueue.swift index 278488b8..40c2ec0f 100644 --- a/TabMail/Services/Sync/BackfillBodyQueue.swift +++ b/TabMail/Services/Sync/BackfillBodyQueue.swift @@ -268,7 +268,8 @@ actor BackfillBodyQueue { try Row.fetchAll(db, sql: """ SELECT id, accountId, folderPath, messageId, isInInbox FROM messageHeader - WHERE headerComplete = 1 AND bodyComplete = 0 AND bodyEmptyConfirmed = 0 AND isInInbox = 0 + WHERE headerComplete = 1 AND bodyComplete = 0 AND bodyEmptyConfirmed = 0 + AND bodyIndexingFailureReason IS NULL AND isInInbox = 0 ORDER BY date DESC """) .map { row in @@ -569,7 +570,32 @@ actor BackfillBodyQueue { } catch { let desc = "\(error)" - if desc.contains("PayloadTooLargeError") { + if let providerError = error as? ProviderError, + case .bodyIndexingUnsupported( + let messageId, + let observedUidValidity, + let fetchedRfc822MessageId + ) = providerError, + let failedItem = items.first(where: { $0.messageId == messageId }) { + let processorItem = BodyFetchProcessor.Item( + headerId: failedItem.headerId, + accountId: failedItem.accountId, + folderPath: failedItem.folderPath, + messageId: failedItem.messageId, + isInInbox: failedItem.isInInbox + ) + let terminalized = await BodyFetchProcessor.markBodyUnindexed( + item: processorItem, + reason: .partialFetchUnsupported, + observedUidValidity: observedUidValidity, + fetchedRfc822MessageId: fetchedRfc822MessageId + ) + print("[BackfillBody] Partial fetch unsupported — \(terminalized ? "recorded terminal-unindexed state" : "row changed; retrying")") + self.batchItemDone(item: failedItem, shouldRetry: !terminalized) + for item in items where item.headerId != failedItem.headerId { + self.batchItemDone(item: item, shouldRetry: true) + } + } else if desc.contains("PayloadTooLargeError") { // Defer a genuinely single oversized item WITHOUT marking it // empty; a multi-item batch isolates its members so a later // dispatch slices each one singly. Keys on THIS batch's actual @@ -1037,7 +1063,8 @@ actor BackfillBodyQueue { try Row.fetchAll(db, sql: """ SELECT id, accountId, folderPath, messageId, isInInbox FROM messageHeader - WHERE headerComplete = 1 AND bodyComplete = 0 AND bodyEmptyConfirmed = 0 AND isInInbox = 0 + WHERE headerComplete = 1 AND bodyComplete = 0 AND bodyEmptyConfirmed = 0 + AND bodyIndexingFailureReason IS NULL AND isInInbox = 0 ORDER BY date DESC """) .map { row in diff --git a/TabMail/Services/Sync/BodyFetchProcessor.swift b/TabMail/Services/Sync/BodyFetchProcessor.swift index 7c23908e..9f9fcefc 100644 --- a/TabMail/Services/Sync/BodyFetchProcessor.swift +++ b/TabMail/Services/Sync/BodyFetchProcessor.swift @@ -31,6 +31,92 @@ enum BodyFetchProcessor { case payloadTooLarge } + /// Retire a deterministic background-indexing failure without claiming the + /// body was empty or indexed. Returns false when the row moved or could not + /// be corroborated, in which case the queue must retry instead of stamping a + /// stale address. + static func markBodyUnindexed( + item: Item, + reason: BodyIndexingFailureReason, + observedUidValidity: Int?, + fetchedRfc822MessageId: String? + ) async -> Bool { + do { + let refusal = try await AppDatabase.dbPool.write { + db -> BodyAddressGate.Refusal? in + guard let header = try MessageHeader.fetchOne(db, key: item.headerId), + let account = try Account.fetchOne(db, key: item.accountId) else { + return .verificationUnavailable + } + guard header.folderPath == item.folderPath, + header.messageId == item.messageId else { + return .fetchProvenanceMismatch + } + let hasComparableEpochs = observedUidValidity != nil + && header.observedUidValidity != nil + let epochMatches = hasComparableEpochs + && header.observedUidValidity == observedUidValidity + let identityMatches: Bool = { + guard let stored = header.rfc822MessageId, + let fetched = fetchedRfc822MessageId, + !stored.isEmpty, !fetched.isEmpty else { return false } + return EmailFilter.normalizeMessageId(stored) + == EmailFilter.normalizeMessageId(fetched) + }() + // A folder-local UID is not identity. Terminalization is + // irreversible automatic state, so require positive proof from + // the exact failed fetch: its SELECT epoch or returned Message-ID. + // Message-ID is fallback proof only when one side lacks epoch + // evidence. An explicit epoch contradiction is stronger than a + // matching, potentially duplicated Message-ID. + guard epochMatches || (!hasComparableEpochs && identityMatches) else { + return .verificationUnavailable + } + if let refusal = BodyAddressGate.refusal( + id: header.id, + accountId: item.accountId, + folderPath: header.folderPath, + messageId: header.messageId, + provider: account.provider, + storedRfc822MessageId: header.rfc822MessageId, + fetchedRfc822MessageId: fetchedRfc822MessageId + ) { + return refusal + } + try db.execute( + sql: """ + UPDATE messageHeader + SET bodyIndexingFailureReason = ?, + bodyComplete = 0, + bodyEmptyConfirmed = 0 + WHERE id = ? AND accountId = ? AND folderPath = ? AND messageId = ? + AND headerComplete = 1 + AND bodyComplete = 0 + AND bodyEmptyConfirmed = 0 + AND bodyIndexingFailureReason IS NULL + """, + arguments: [ + reason.rawValue, item.headerId, item.accountId, + item.folderPath, item.messageId, + ] + ) + return db.changesCount == 1 ? nil : .verificationUnavailable + } + if let refusal { + BackgroundSyncLogger.log( + "[BodyFetch] REFUSED terminal-unindexed write — \(refusal.logDescription); retrying" + ) + return false + } + return true + } catch { + if !error.isDatabaseSuspensionAbort { + print("[BodyFetch] Failed to record terminal-unindexed state: \(error)") + } + return false + } + } + /// Fetch phase: provider.fetchMessage + render body. Provider-bound (network I/O). /// Returns the rendered MessageBody and extracted plain text, or an error result. struct FetchResult: Sendable { @@ -101,7 +187,11 @@ enum BodyFetchProcessor { } let fetchAttachment = buildAttachmentFetcher( - accountId: item.accountId, messageId: item.messageId, folderPath: item.folderPath + accountId: item.accountId, + messageId: item.messageId, + folderPath: item.folderPath, + expectedObservedUidValidity: fullMessage.observedUidValidity, + expectedRfc822MessageId: fullMessage.header.rfc822MessageId ) let (renderedBody, plainText, hasUnresolvedICS) = await renderBody( headerId: item.headerId, @@ -129,6 +219,18 @@ enum BodyFetchProcessor { fetchedRfc822MessageId: fullMessage.header.rfc822MessageId )) } catch { + if let providerError = error as? ProviderError, + case .bodyIndexingUnsupported( + _, let observedUidValidity, let fetchedRfc822MessageId + ) = providerError { + _ = await markBodyUnindexed( + item: item, + reason: .partialFetchUnsupported, + observedUidValidity: observedUidValidity, + fetchedRfc822MessageId: fetchedRfc822MessageId + ) + return .failure(.retry) + } let desc = "\(error)" if desc.contains("PayloadTooLargeError") { // Data-integrity rule 1 ("NEVER mark unfetched content as fetched"): @@ -344,6 +446,7 @@ enum BodyFetchProcessor { UPDATE messageHeader SET bodyEmptyConfirmed = 1, bodyComplete = 1, + bodyIndexingFailureReason = NULL, emptyFetchCount = emptyFetchCount + 1, summaryBlurb = 'This message has no content.', actionTag = ?, @@ -417,6 +520,7 @@ enum BodyFetchProcessor { END, bodyComplete = 0, bodyEmptyConfirmed = 0, + bodyIndexingFailureReason = NULL, emptyFetchCount = 0, embeddingComplete = 0 WHERE id = ? @@ -496,7 +600,7 @@ enum BodyFetchProcessor { // misses against this header (e.g. IMAP flap). Now that we have real // content, the miss chain is broken — start counting fresh next time. try db.execute( - sql: "UPDATE messageHeader SET snippet = ?, bodyComplete = 1, missFetchCount = 0 WHERE id = ?", + sql: "UPDATE messageHeader SET snippet = ?, bodyComplete = 1, bodyIndexingFailureReason = NULL, missFetchCount = 0 WHERE id = ?", arguments: [item.snippet, item.headerId] ) } @@ -632,7 +736,11 @@ enum BodyFetchProcessor { let t0 = CFAbsoluteTimeGetCurrent() let fetchAttachment = buildAttachmentFetcher( - accountId: item.accountId, messageId: item.messageId, folderPath: item.folderPath + accountId: item.accountId, + messageId: item.messageId, + folderPath: item.folderPath, + expectedObservedUidValidity: fullMessage.observedUidValidity, + expectedRfc822MessageId: fullMessage.header.rfc822MessageId ) let (renderedBody, plainText, hasUnresolvedICS) = await renderBody( headerId: item.headerId, @@ -697,10 +805,14 @@ enum BodyFetchProcessor { fetchAttachment: (@Sendable (String, String?) async throws -> Data)? = nil ) async -> (body: MessageBody, plainText: String?, hasUnresolvedICS: Bool) { // Convert main-app FullMessageInfo → shared RawBodyIngredients. - let sharedAttachments = fullMessage.attachments.map { - AttachmentRef( - filename: $0.filename, contentType: $0.contentType, - section: $0.section, size: $0.size, encoding: $0.encoding + let sharedAttachments = fullMessage.attachments.compactMap { attachment -> AttachmentRef? in + if let allowed = fullMessage.renderIngredientSections, + !allowed.contains(attachment.section) { + return nil + } + return AttachmentRef( + filename: attachment.filename, contentType: attachment.contentType, + section: attachment.section, size: attachment.size, encoding: attachment.encoding ) } let sharedInlineImages = fullMessage.inlineImages.map { @@ -759,7 +871,11 @@ enum BodyFetchProcessor { // MARK: - Helpers private static func buildAttachmentFetcher( - accountId: String, messageId: String, folderPath: String + accountId: String, + messageId: String, + folderPath: String, + expectedObservedUidValidity: Int?, + expectedRfc822MessageId: String? ) -> @Sendable (String, String?) async throws -> Data { return { section, encoding in let queue = await AccountManager.shared.workQueues[accountId] @@ -768,7 +884,14 @@ enum BodyFetchProcessor { return try await queue.execute(priority: .bodyFetch) { if let imap = provider as? IMAPProvider { - return try await imap.fetchAttachment(messageId: messageId, folder: folderPath, section: section, encoding: encoding) + return try await imap.fetchAttachment( + messageId: messageId, + folder: folderPath, + section: section, + encoding: encoding, + expectedObservedUidValidity: expectedObservedUidValidity, + expectedRfc822MessageId: expectedRfc822MessageId + ) } else if let gmail = provider as? GmailProvider { return try await gmail.fetchAttachment(messageId: messageId, attachmentId: section) } else if let exchange = provider as? ExchangeProvider { diff --git a/TabMail/Services/Sync/SyncEngineBackfill.swift b/TabMail/Services/Sync/SyncEngineBackfill.swift index bb4d09b7..c474afd3 100644 --- a/TabMail/Services/Sync/SyncEngineBackfill.swift +++ b/TabMail/Services/Sync/SyncEngineBackfill.swift @@ -174,9 +174,17 @@ extension SyncEngine { Column("backfillPageToken").set(to: nil as String?) ) } - // Reset bodyEmptyConfirmed — gives previously-empty messages a fresh chance + // Reset body terminal states — gives previously-empty and previously + // server-incompatible messages a fresh chance after an app/server update. try? await AppDatabase.backgroundPool.write { db in - try db.execute(sql: "UPDATE messageHeader SET bodyEmptyConfirmed = 0, emptyFetchCount = 0, bodyComplete = 0 WHERE bodyEmptyConfirmed = 1") + try db.execute(sql: """ + UPDATE messageHeader + SET bodyEmptyConfirmed = 0, + bodyIndexingFailureReason = NULL, + emptyFetchCount = 0, + bodyComplete = 0 + WHERE bodyEmptyConfirmed = 1 OR bodyIndexingFailureReason IS NOT NULL + """) } // Reset cc/bcc backfill flag so existing messages get cc/bcc populated UserDefaults.standard.set(false, forKey: "ccBccBackfillDone") @@ -325,7 +333,7 @@ extension SyncEngine { let headersDone = incompleteFolders == 0 // GRDB is sole authority for body status flags - let (grdbTotal, grdbIndexed, pendingBody) = (try? await dbPool.read { db -> (Int, Int, Int) in + let (grdbTotal, grdbIndexed, pendingBody, unindexedBody) = (try? await dbPool.read { db -> (Int, Int, Int, Int) in let total = try MessageHeader.filter(Column("accountId") == accountId).fetchCount(db) let indexed = try MessageHeader.filter( Column("accountId") == accountId && @@ -339,10 +347,15 @@ extension SyncEngine { Column("accountId") == accountId && Column("headerComplete") == true && Column("bodyComplete") == false && - Column("bodyEmptyConfirmed") == false + Column("bodyEmptyConfirmed") == false && + Column("bodyIndexingFailureReason") == nil ).fetchCount(db) - return (total, indexed, pending) - }) ?? (0, 0, 1) // pending=1 on read failure → never false-complete + let unindexed = try MessageHeader.filter( + Column("accountId") == accountId && + Column("bodyIndexingFailureReason") != nil + ).fetchCount(db) + return (total, indexed, pending, unindexed) + }) ?? (0, 0, 1, 0) // pending=1 on read failure → never false-complete // Gmail/Exchange: uidTotal is 0 (no IMAP UIDs). Use the server-reported // total as the denominator ONLY WHILE STILL CRAWLING, so the bar reflects @@ -382,7 +395,8 @@ extension SyncEngine { headersDone: headersDone, isPaused: false, totalEmails: totalEmails, ftsIndexed: ftsIndexed, uidTotal: uidTotal, uidWalked: uidWalked, - pendingBodyCount: pendingBody + pendingBodyCount: pendingBody, + unindexedBodyCount: unindexedBody ) } diff --git a/TabMail/Services/Sync/SyncEngineFTS.swift b/TabMail/Services/Sync/SyncEngineFTS.swift index 9574badc..9bf31cee 100644 --- a/TabMail/Services/Sync/SyncEngineFTS.swift +++ b/TabMail/Services/Sync/SyncEngineFTS.swift @@ -172,6 +172,7 @@ extension SyncEngine { try String.fetchAll(db, sql: """ SELECT id FROM messageHeader WHERE headerComplete = 1 AND bodyComplete = 0 AND bodyEmptyConfirmed = 0 + AND bodyIndexingFailureReason IS NULL LIMIT 5000 """) } @@ -291,12 +292,14 @@ extension SyncEngine { return try String.fetchAll(db, sql: """ SELECT id FROM messageHeader WHERE headerComplete = 1 AND bodyComplete = 0 AND bodyEmptyConfirmed = 0 + AND bodyIndexingFailureReason IS NULL AND id LIKE ? """, arguments: ["\(prefix)%"]) } return try String.fetchAll(db, sql: """ SELECT id FROM messageHeader WHERE headerComplete = 1 AND bodyComplete = 0 AND bodyEmptyConfirmed = 0 + AND bodyIndexingFailureReason IS NULL """) } var healed = 0 diff --git a/TabMail/ViewModels/MessageDetailViewModel.swift b/TabMail/ViewModels/MessageDetailViewModel.swift index fe96e4f4..dbf2cbe0 100644 --- a/TabMail/ViewModels/MessageDetailViewModel.swift +++ b/TabMail/ViewModels/MessageDetailViewModel.swift @@ -1007,6 +1007,7 @@ final class MessageDetailViewModel { func startBodyPoll() { bodyPollTask?.cancel() bodyPollTask = Task { [weak self] in + if let self, await self.stopForTerminalBodyFailure() { return } // IMMEDIATE cache check, BEFORE the first 2s sleep. On the // notification-tap deep-link path the body is usually ALREADY in the DB // (the deep-link's own NSE merge wrote it) — loadBody just got cancelled @@ -1038,6 +1039,7 @@ final class MessageDetailViewModel { while !Task.isCancelled { try? await Task.sleep(for: .seconds(2)) guard !Task.isCancelled, let self else { return } + if await self.stopForTerminalBodyFailure() { return } if self.message == nil { await self.recoverHeaderIfMissing() } guard self.messageBody == nil else { // Body already landed (entry fast-path above, merge-commit @@ -1148,6 +1150,33 @@ final class MessageDetailViewModel { } } + /// Stop an open detail view once the durable row says automatic bounded + /// body retrieval is unsupported. This is checked both before polling and + /// on every tick, covering a row terminalized by the background queue while + /// the view is already open. Smart Reindex clears the reason for a new try. + @MainActor + @discardableResult + func stopForTerminalBodyFailure() async -> Bool { + let headerId = resolvedId + let rawReason = try? await dbPool.pool.read { db in + try String.fetchOne( + db, + sql: "SELECT bodyIndexingFailureReason FROM messageHeader WHERE id = ?", + arguments: [headerId] + ) + } + guard rawReason.flatMap(BodyIndexingFailureReason.init(rawValue:)) != nil else { + return false + } + isLoading = false + error = ProviderError.bodyIndexingUnsupported( + messageId: message?.messageId ?? "", + observedUidValidity: nil, + fetchedRfc822MessageId: nil + ).localizedDescription + return true + } + /// Explicit user Retry from the Not-Found screen. `loadBody()` alone is a /// permanent no-op after a failed first run — the latches below memoize /// the failure and each must be reset for the retry to actually re-run: @@ -1384,6 +1413,7 @@ final class MessageDetailViewModel { loadThreadMessagesAsync() return } + if await stopForTerminalBodyFailure() { return } // Address-corroboration pre-gate (`BodyAddressGate`). `optimisticMoveToFolder` // leaves the row at (destination folder, SOURCE UID) with a nil epoch until the // drain's `finishMove` re-keys it — and on IMAP that address names a DIFFERENT @@ -1456,7 +1486,9 @@ final class MessageDetailViewModel { // or any transient failure. A reconnect or background path may still // write the body to DB later. if messageBody == nil { - startBodyPoll() + if !(await stopForTerminalBodyFailure()) { + startBodyPoll() + } } } diff --git a/TabMail/Views/Message/AttachmentListView.swift b/TabMail/Views/Message/AttachmentListView.swift index d9836373..6a134c0f 100644 --- a/TabMail/Views/Message/AttachmentListView.swift +++ b/TabMail/Views/Message/AttachmentListView.swift @@ -6,20 +6,103 @@ import SwiftUI import QuickLook import UIKit +enum EmlAttachmentPreviewLoader { + struct Payload: Sendable { + let html: String + let nestedAttachments: [AttachmentInfo] + } + + enum LoadError: LocalizedError { + case invalidMessage + + var errorDescription: String? { + "The attached email could not be opened." + } + } + + /// The tap-time path for metadata-only `.eml` attachments. The caller + /// supplies the provider fetch so tests can drive the exact same loader + /// against a wire server without rendering SwiftUI. + static func load( + attachment: AttachmentInfo, + fetch: @Sendable () async throws -> Data + ) async throws -> Payload { + let bytes = try await fetch() + // Provider cancellation is cooperative and a socket read may still + // return bytes after the view that initiated it has disappeared. Stop + // before parsing/presentation so a torn-down caller cannot acquire the + // global PreviewFreezeGate with no live sheet left to release it. + try Task.checkCancellation() + guard let parsed = EmlParsing.parse(rawBytes: bytes) else { + throw LoadError.invalidMessage + } + let html = EmlMarker.build( + filename: attachment.filename, + partSection: attachment.section, + envelope: parsed.envelope, + bodyHtml: parsed.bodyHtml + ) + let nested = parsed.nested.enumerated().map { index, metadata in + AttachmentInfo( + filename: metadata.filename, + contentType: metadata.contentType, + section: EmlParsing.nestedSection(parent: attachment.section, index: index), + size: metadata.size, + encoding: attachment.encoding, + parentEmlSection: attachment.section + ) + } + return Payload(html: html, nestedAttachments: nested) + } + + /// Acquire the global preview freeze only while the retained presentation + /// task is still live. Keeping the cancellation check and acquisition in + /// one synchronous MainActor operation closes the post-load teardown race. + @MainActor + static func beginPreviewFreeze() throws { + try Task.checkCancellation() + PreviewFreezeGate.shared.begin() + } +} + +/// View-lifecycle owner for the one in-flight `.eml` presentation task. The +/// SwiftUI disappearance hook and tests use the same cancellation boundary. +@MainActor +final class EmlAttachmentPreviewTaskCoordinator { + private var task: Task? + + @discardableResult + func start( + _ operation: @escaping @MainActor () async -> Void + ) -> Task { + cancel() + let next = Task { @MainActor in + await operation() + } + task = next + return next + } + + func cancel() { + task?.cancel() + task = nil + } +} + struct AttachmentListView: View { let message: MessageHeader let attachments: [AttachmentInfo] - /// Rendered HTML of the parent message (MessageBody.htmlContent). Used to power - /// `.eml` attachment previews — the embedded email is already inside this string - /// as a `
` marker, so the preview - /// sheet just re-renders it with preview-mode CSS. `nil` means .eml taps fall - /// back to the old QuickLook flow (which shows a file, not a rendered email). + /// Retained for source compatibility with existing call sites. `.eml` payloads + /// are metadata-only during background indexing and are always fetched through + /// the bounded attachment path when tapped; parent HTML is never treated as the + /// attached message's bytes. let bodyHtml: String? @State private var downloadingSection: String? @State private var downloadedFiles: [String: URL] = [:] @State private var emlPreview: EmlPreviewState? @State private var error: String? + @State private var emlDownloadCoordinator = EmlAttachmentPreviewTaskCoordinator() private let manager = AccountManager.shared @@ -63,20 +146,8 @@ struct AttachmentListView: View { Button { if attachment.contentType.lowercased().contains("text/calendar") { downloadAndImportICS(attachment) - } else if isEmlAttachment(attachment), let html = bodyHtml { - // .eml has no QuickLook renderer — use our own sheet that - // re-renders the already-stored body HTML with preview-mode - // CSS showing only this attachment's section. Pass along - // the nested attachments (filtered by parentEmlSection - // matching this .eml's section) so the preview sheet can - // surface them as a mini attachment strip. - PreviewFreezeGate.shared.begin() - let nested = attachments.filter { $0.parentEmlSection == attachment.section } - emlPreview = EmlPreviewState( - html: html, - filename: attachment.filename, - nestedAttachments: nested - ) + } else if isEmlAttachment(attachment) { + downloadAndPreviewEml(attachment) } else if let existing = downloadedFiles[attachment.section] { // Imperative QuickLook — detached from this re-rendering // List row (see AttachmentQuickLook). It raises the @@ -164,6 +235,7 @@ struct AttachmentListView: View { } } .onDisappear { + emlDownloadCoordinator.cancel() // Safety net: if the view tears down with the .eml sheet still // presented, release the gate so the app doesn't stay frozen. The // QuickLook path is imperative (AttachmentQuickLook owns its own gate @@ -180,6 +252,38 @@ struct AttachmentListView: View { } } + private func downloadAndPreviewEml(_ attachment: AttachmentInfo) { + downloadingSection = attachment.section + error = nil + emlDownloadCoordinator.start { + do { + let payload = try await EmlAttachmentPreviewLoader.load( + attachment: attachment + ) { + try await manager.fetchAttachment( + for: message, + section: attachment.section, + encoding: attachment.encoding + ) + } + try EmlAttachmentPreviewLoader.beginPreviewFreeze() + emlPreview = EmlPreviewState( + html: payload.html, + filename: attachment.filename, + nestedAttachments: payload.nestedAttachments + ) + } catch is CancellationError { + // Navigation teardown owns cancellation; it is not a user- + // visible download failure and must never acquire the gate. + } catch { + self.error = SyncEngine.isConnectionError(error) + ? "Download failed. Check your connection and try again." + : error.localizedDescription + } + downloadingSection = nil + } + } + /// ⚠️ **Deliberately NOT gated on `AttachmentFilename.isSafeFileComponent`, and /// that asymmetry with `downloadAndPreview` below is the intended behaviour.** /// diff --git a/TabMail/Views/Message/EmlAttachmentPreview.swift b/TabMail/Views/Message/EmlAttachmentPreview.swift index 223dde24..ce6c7217 100644 --- a/TabMail/Views/Message/EmlAttachmentPreview.swift +++ b/TabMail/Views/Message/EmlAttachmentPreview.swift @@ -9,9 +9,10 @@ import QuickLook /// /// The envelope (subject, from, date, to/cc) is rendered natively in SwiftUI from /// `data-*` attributes on the `.tm-eml-section` marker — no HTML duplicate. The -/// body is the already-stored `MessageBody.htmlContent`, re-rendered by the same -/// `AutoSizingHTMLView` used for the main message (quote-collapse, dark-mode, -/// cleanup-JS, and table-block CSS all apply identically). No re-fetch, no re-parse. +/// body is parsed at tap time from bytes fetched through the provider's bounded +/// attachment path, then rendered by the same `AutoSizingHTMLView` used for the +/// main message (quote-collapse, dark-mode, cleanup-JS, and table-block CSS all +/// apply identically). /// /// Nested attachments (files inside the `.eml`, e.g. a PDF attached to a forwarded /// email) are surfaced HERE — not in the parent message's attachment list — so the @@ -62,25 +63,11 @@ struct EmlAttachmentPreview: View { // "local assets are unavailable": `bodyContentKey` is deliberately // nil, so no `BodyAssetSchemeHandler` is registered. // - // ⚠️ MEASURED 2026-08-12, and it CONTRADICTS the plan's grading. §10.1 - // C5 graded this path "LIKELY-provenance-free — the attachment's own - // bytes, not the persisted parent body — confirm during - // implementation". Confirmed FALSE: `AttachmentListView` builds - // `EmlPreviewState(html: bodyHtml, …)` from its own `bodyHtml` - // parameter, and both `AttachmentListView(...)` call sites in - // `MessageCardView` pass `bodyHtml: body.htmlContent` — the persisted - // parent `MessageBody.htmlContent`. This sheet re-renders the PARENT - // body with preview-mode CSS that shows only the selected `.eml` - // section; it never parses the attachment's bytes. So it is - // provenance-BEARING and can carry `tabmail-asset://` refs. - // - // That is not a P1d regression either: this call site has never passed - // a headerId, so no handler was registered here on shipped code and - // those refs already fail closed — the same pre-existing shape as the - // compose quote (§11.1). Carrying the parent's `MessageBody.id` here - // would CHANGE behaviour (broken images would start loading), which - // the standing "no behaviour changes, just security" directive puts - // out of scope for this commit. + // The HTML comes from the attachment's freshly fetched and parsed + // RFC 822 bytes, not the persisted parent body. No body content key + // is available for those standalone bytes, so local assets remain + // deliberately unavailable and `tabmail-asset://` references fail + // closed. AutoSizingHTMLView(html: html, previewFilename: filename) if !nestedAttachments.isEmpty { nestedAttachmentStrip diff --git a/TabMail/Views/Settings/FastSyncView.swift b/TabMail/Views/Settings/FastSyncView.swift index ec882110..f4385954 100644 --- a/TabMail/Views/Settings/FastSyncView.swift +++ b/TabMail/Views/Settings/FastSyncView.swift @@ -12,92 +12,16 @@ struct FastSyncView: View { @State private var refreshTick = 0 private let refreshTimer = Timer.publish(every: 2, on: .main, in: .common).autoconnect() - // MARK: - Keep-awake state - - /// Polled snapshots of each body queue's idle state (`ActiveBodyQueue.isIdle` / - /// `BackfillBodyQueue.isIdle` — queue empty AND no active batch). Start `false` - /// so the screen holds awake until the first poll confirms idle (fail-safe: an - /// un-polled screen never sleeps mid-fetch). The keep-awake lock follows the - /// queues' RUNNABLE state directly — an oversized-only-incomplete account no - /// longer pins the device awake, because `handlePayloadTooLarge` takes the - /// quarantined item out of the queue (`QueueStorage.removeFromQueue`) and - /// `admit` never re-admits it, so the queues go idle. - /// - /// Deliberately NO repopulation on poll and no admission latch: re-running both - /// full work-remaining scans every poll tick - /// (`BackfillBodyQueue.repopulateFromDatabase` is a ~200K-row scan) would drain - /// CPU/DB — the opposite of this screen's battery goal — and the - /// "repopulation-not-yet-run → lock releases" edge is acceptable (the device - /// sleeps; bodies fetch on the next foreground, no data loss). `SyncScheduler` - /// owns (re)populating the queues on wake. - @State private var activeBodyIdle = false - @State private var backfillBodyIdle = false - /// True when all accounts have progress AND all are fully complete. /// - /// This is a TRUTH CLAIM about the mailbox and deliberately stays gated on - /// `BackfillProgress.isFullyComplete` (`pendingBodyCount == 0`): an account - /// holding a quarantined oversized body genuinely does not have every body - /// indexed, so the "Sync Complete" label stays withheld rather than lying. - /// Only the wake lock moved to the runnable-state predicate below. + /// `BackfillProgress.isFullyComplete` means the header walk ended and no body + /// remains runnable. A separate `unindexedBodyCount` keeps terminal failures + /// visible, so completion never implies that every body reached FTS. private var isAllComplete: Bool { let values = Array(state.backfillProgressByAccount.values) return !values.isEmpty && values.allSatisfy(\.isFullyComplete) } - /// Keep-awake predicate. The device stays awake while there is RUNNABLE body - /// work, NOT while durable completeness is < 100%. - /// - /// `ActiveBodyQueue.handlePayloadTooLarge` / `BackfillBodyQueue - /// .handlePayloadTooLarge` leave an oversized (`PayloadTooLargeError`) row - /// honestly `bodyComplete = 0 / bodyEmptyConfirmed = 0` — the body demonstrably - /// exists, it merely did not fit — so `BackfillProgress.pendingBodyCount` keeps - /// counting it and never reaches 0 for that account. The old - /// `keepScreenAwake(while: !isAllComplete)` gate therefore pinned the wake lock - /// indefinitely on any account holding a single oversized message. This - /// predicate instead follows the header walk plus the two body queues' idle - /// state, releasing once the walk is complete and the queues drain — the - /// quarantined rows are `removeFromQueue`'d and never re-admitted, so an - /// oversized-only remainder goes idle. Pure and `nonisolated` so it is - /// assertable without driving SwiftUI. Holds awake when: - /// - any account's header walk is not done (`headersDone != true`, including a - /// missing progress entry — mapped to `false` by the caller), OR - /// - either body queue is non-idle (queued / in-flight / active batch). - nonisolated static func keepScreenAwakeWhileWorking( - accountHeadersDone: [Bool], - activeBodyIdle: Bool, - backfillBodyIdle: Bool - ) -> Bool { - if accountHeadersDone.contains(false) { return true } - if !activeBodyIdle { return true } - if !backfillBodyIdle { return true } - return false - } - - /// Live keep-awake value — maps the current SwiftUI state into the pure - /// predicate. `state.backfillProgressByAccount[$0.id]?.headersDone == true` - /// preserves the `?.headersDone != true` semantics (a missing progress entry ⇒ - /// not-done ⇒ hold awake). - private var holdAwake: Bool { - Self.keepScreenAwakeWhileWorking( - accountHeadersDone: navigationStore.accounts.map { - state.backfillProgressByAccount[$0.id]?.headersDone == true - }, - activeBodyIdle: activeBodyIdle, - backfillBodyIdle: backfillBodyIdle - ) - } - - /// Poll both body queues' idle state into `@State` for the keep-awake predicate. - /// No repopulation here — `SyncScheduler` owns (re)populating the queues on - /// foreground/wake; this screen only OBSERVES their runnable state so the wake - /// lock releases once the header walk is done and the queues drain (the - /// quarantined oversized rows having been removed from the queue). - private func refreshBodyQueueIdleState() async { - activeBodyIdle = await ActiveBodyQueue.shared.isIdle - backfillBodyIdle = await BackfillBodyQueue.shared.isIdle - } - private func formatETA(_ seconds: Double) -> String { if seconds < 60 { return "<1 min" } if seconds < 3600 { return "\(Int(seconds / 60)) min" } @@ -141,13 +65,14 @@ struct FastSyncView: View { if !allProgress.isEmpty { let totalEmails = allProgress.reduce(0) { $0 + $1.totalEmails } let totalIndexed = allProgress.reduce(0) { $0 + $1.ftsIndexed } + let totalUnindexed = allProgress.reduce(0) { $0 + $1.unindexedBodyCount } let totalUidScope = allProgress.reduce(0) { $0 + $1.uidTotal } let totalUidWalked = allProgress.reduce(0) { $0 + $1.uidWalked } let allHeadersDone = allProgress.allSatisfy(\.headersDone) // Show UID progress only while actively walking (not 100% AND not all done). // Once UIDs hit 100% or headers are done, switch to FTS indexing view. let showUidWalk = !allHeadersDone && totalUidScope > 0 && totalUidWalked < totalUidScope - let fraction: Double = showUidWalk + let fraction: Double = isAllComplete ? 1.0 : showUidWalk ? min(1.0, Double(totalUidWalked) / Double(totalUidScope)) : (totalEmails > 0 ? min(1.0, Double(totalIndexed) / Double(totalEmails)) : 0.0) @@ -160,6 +85,13 @@ struct FastSyncView: View { Text("\(totalUidWalked.formatted()) / \(totalUidScope.formatted()) UIDs walked (\(pct)%)") .font(.caption) .foregroundStyle(.secondary) + } else if let terminalText = BodyIndexingProgressText.terminalCompletion( + isComplete: isAllComplete, + unindexedCount: totalUnindexed + ) { + Text(terminalText) + .font(.caption) + .foregroundStyle(.secondary) } else if totalEmails > 0 { let pct = Int(Double(totalIndexed) / Double(totalEmails) * 100) Text("\(totalIndexed.formatted()) / \(totalEmails.formatted()) indexed (\(pct)%)") @@ -211,7 +143,13 @@ struct FastSyncView: View { // Completion state if isAllComplete { - Label("Sync Complete", systemImage: "checkmark.circle.fill") + let totalUnindexed = state.backfillProgressByAccount.values.reduce(0) { + $0 + $1.unindexedBodyCount + } + Label( + BodyIndexingProgressText.completion(unindexedCount: totalUnindexed), + systemImage: "checkmark.circle.fill" + ) .font(.headline) .foregroundStyle(.green) } @@ -238,7 +176,7 @@ struct FastSyncView: View { } } } - .keepScreenAwake(while: holdAwake) + .keepScreenAwake(while: true) .onAppear { Task { await manager.setFastSyncMode(true) } Task { @@ -250,8 +188,6 @@ struct FastSyncView: View { await manager.syncEngine.startBackfill(account: account) } } - // Poll queue idle state for the keep-awake predicate. - Task { await refreshBodyQueueIdleState() } } .onDisappear { Task { await manager.setFastSyncMode(false) } } .onReceive(refreshTimer) { _ in @@ -262,9 +198,6 @@ struct FastSyncView: View { await manager.syncEngine.updateBackfillProgressForAccount(account) } } - // Re-poll queue idle state so the keep-awake lock releases once the walk - // is done and the queues drain (quarantined oversized rows removed). - Task { await refreshBodyQueueIdleState() } } } } @@ -291,7 +224,10 @@ private struct FastSyncAccountCard: View { } if let progress { if progress.isFullyComplete { - Text("\(progress.totalEmails.formatted()) messages indexed") + Text(BodyIndexingProgressText.terminalCompletion( + isComplete: progress.isFullyComplete, + unindexedCount: progress.unindexedBodyCount + ) ?? "\(progress.totalEmails.formatted()) messages indexed") .font(.caption2) .foregroundStyle(.secondary) } else { diff --git a/TabMail/Views/Settings/SettingsView.swift b/TabMail/Views/Settings/SettingsView.swift index fdc3106d..86066573 100644 --- a/TabMail/Views/Settings/SettingsView.swift +++ b/TabMail/Views/Settings/SettingsView.swift @@ -294,6 +294,7 @@ struct SettingsView: View { let allProgress = Array(state.backfillProgressByAccount.values) let totalEmails = allProgress.reduce(0) { $0 + $1.totalEmails } let totalIndexed = allProgress.reduce(0) { $0 + $1.ftsIndexed } + let totalUnindexed = allProgress.reduce(0) { $0 + $1.unindexedBodyCount } let isComplete = !allProgress.isEmpty && allProgress.allSatisfy(\.isFullyComplete) if !allProgress.isEmpty { @@ -301,7 +302,7 @@ struct SettingsView: View { let totalUidWalked = allProgress.reduce(0) { $0 + $1.uidWalked } let allHeadersDone = allProgress.allSatisfy(\.headersDone) // During header walk: show UID progress. After: show FTS indexing. - let fraction: Double = (!allHeadersDone && totalUidScope > 0) + let fraction: Double = isComplete ? 1.0 : (!allHeadersDone && totalUidScope > 0) ? min(1.0, Double(totalUidWalked) / Double(totalUidScope)) : (totalEmails > 0 ? min(1.0, Double(totalIndexed) / Double(totalEmails)) : 0.0) VStack(alignment: .leading, spacing: 6) { @@ -313,6 +314,13 @@ struct SettingsView: View { Text("\(totalUidWalked.formatted()) / \(totalUidScope.formatted()) UIDs (\(pct)%)") .font(.caption) .foregroundStyle(.secondary) + } else if let terminalText = BodyIndexingProgressText.terminalCompletion( + isComplete: isComplete, + unindexedCount: totalUnindexed + ) { + Text(terminalText) + .font(.caption) + .foregroundStyle(.secondary) } else if totalEmails > 0 { let pct = Int(Double(totalIndexed) / Double(totalEmails) * 100) Text("\(totalIndexed.formatted()) / \(totalEmails.formatted()) indexed (\(pct)%)") diff --git a/TabMailNotificationService/NSEIMAPConnection.swift b/TabMailNotificationService/NSEIMAPConnection.swift index e0fa786b..f5c7b4d6 100644 --- a/TabMailNotificationService/NSEIMAPConnection.swift +++ b/TabMailNotificationService/NSEIMAPConnection.swift @@ -15,9 +15,9 @@ import SwiftMail /// Memory notes: /// • SwiftMail pulls in SwiftNIO + NIOSSL. Static-link overhead ~2–4 MB /// per architecture. Per-live-connection RSS ~1–2 MB steady-state. -/// • We SELECT a single mailbox (INBOX) and issue a single UID SEARCH + -/// UID FETCH. No BODYSTRUCTURE walking beyond what SwiftMail does -/// internally for `fetchMessage(from:)`. +/// • We SELECT a single mailbox (INBOX), issue one UID SEARCH and one +/// BODYSTRUCTURE fetch, then fetch only render-required MIME parts in +/// bounded chunks. Normal attachment payloads are never loaded here. /// /// Safety notes: /// • No credentials are ever logged. We redact email/username in the @@ -132,7 +132,10 @@ enum NSEIMAPConnection { // Fetch the one message the push pointed us at. let info: MessageInfo? do { - info = try await server.fetchMessageInfo(for: uid) + info = try await server.fetchMessageInfo( + for: uid, + options: IMAPFetchMapping.bodyFetchMetadataOptions + ) } catch { NSELog.step("NSE IMAP FETCH info failed: \(String(describing: error))") return nil @@ -144,7 +147,28 @@ enum NSEIMAPConnection { let message: Message do { - message = try await server.fetchMessage(from: info) + // Chunking bounds each wire response; it does not bound the encoded + // bytes retained across all parts or the decode/render copies that + // follow. Reuse the ordinary four-MiB response ceiling as the NSE's + // aggregate encoded-body admission budget, leaving the fixed 24-MB + // process envelope unchanged. Unknown sizes fail closed to passive + // notification delivery before any body literal is requested. + guard let parts = try await IMAPFetchMapping.fetchRequiredBodyPartsWithinAggregateBudget( + in: info.parts, + byteBudget: IMAPFetchMapping.responseBufferLimit, + fetchChunk: { part, offset, count in + try await server.fetchPart( + section: part.section, + of: uid, + offset: offset, + count: count + ) + } + ) else { + NSELog.step("NSE IMAP body exceeds bounded memory admission; using passive delivery") + return nil + } + message = Message(header: info, parts: parts) } catch { NSELog.step("NSE IMAP FETCH message failed: \(String(describing: error))") return nil diff --git a/TabMailTests/Database/BodyIndexingFailureMigrationTests.swift b/TabMailTests/Database/BodyIndexingFailureMigrationTests.swift new file mode 100644 index 00000000..0e7955c7 --- /dev/null +++ b/TabMailTests/Database/BodyIndexingFailureMigrationTests.swift @@ -0,0 +1,56 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ + +import GRDB +import Testing +@testable import TabMail + +@Suite("v88 body-indexing terminal state") +struct BodyIndexingFailureMigrationTests { + @Test("v88 adds a nullable reason and the exact partial queue index") + func migrationAddsTerminalReasonAndQueueIndex() throws { + var configuration = Configuration() + configuration.foreignKeysEnabled = true + let database = try DatabaseQueue(configuration: configuration) + var migrator = DatabaseMigrator() + AppDatabase.registerAllMigrations(on: &migrator) + + try migrator.migrate(database, upTo: "v87_retireDirectAIPending") + let before = try database.read { db in + try db.columns(in: "messageHeader").map(\.name) + } + #expect(!before.contains("bodyIndexingFailureReason")) + + try migrator.migrate(database) + let after = try database.read { db in + ( + columns: try db.columns(in: "messageHeader"), + indexSQL: try String.fetchOne( + db, + sql: "SELECT sql FROM sqlite_master WHERE type = 'index' AND name = ?", + arguments: ["messageHeader_bodyIndexingQueue"] + ) + ) + } + let reason = try #require(after.columns.first(where: { + $0.name == "bodyIndexingFailureReason" + })) + #expect(reason.isNotNull == false) + let indexSQL = try #require(after.indexSQL) + #expect(indexSQL.contains("bodyIndexingFailureReason IS NULL")) + #expect(indexSQL.contains("headerComplete = 1")) + } + + @Test("New message rows default to no terminal failure") + func newRowsStartRetryable() throws { + let database = try TestDatabase.make() + try TestDatabase.insertAccount(database) + try TestDatabase.insertFolder(database) + let header = try TestDatabase.insertMessageHeader(database) + let stored = try database.read { db in + try MessageHeader.fetchOne(db, key: header.id) + } + #expect(stored?.bodyIndexingFailureReason == nil) + } +} diff --git a/TabMailTests/Database/DatabaseMigrationTests.swift b/TabMailTests/Database/DatabaseMigrationTests.swift index 5af7f2ce..9aadc0b2 100644 --- a/TabMailTests/Database/DatabaseMigrationTests.swift +++ b/TabMailTests/Database/DatabaseMigrationTests.swift @@ -1053,6 +1053,10 @@ struct V70CrossStoreInvariantTests { #expect(untagged?.actionTagSetAt == nil, "no tag means nothing to stamp") // The going-forward side: v81 relaxes history, not the invariant. + // Bring the fixture to the current schema before inserting the current + // MessageHeader model; later nullable columns do not change v81's stamp + // semantics, but PersistableRecord correctly includes them in INSERTs. + try afterMigrator.migrate(db) var fresh = MessageHeader( messageId: "81-post-upgrade", subject: "s", from: "Sender", fromAddress: "sender@example.com", to: "recipient@example.com", diff --git a/TabMailTests/Database/MigrationForeignKeyModeTests.swift b/TabMailTests/Database/MigrationForeignKeyModeTests.swift index 0a0c4e48..63b778f5 100644 --- a/TabMailTests/Database/MigrationForeignKeyModeTests.swift +++ b/TabMailTests/Database/MigrationForeignKeyModeTests.swift @@ -29,14 +29,14 @@ import GRDB /// (`MIS-033`), rather than trusting any integer here: /// ``` /// rg -c --pcre2 '^(?!\s*(///|//)).*foreignKeyChecks: \.immediate' \ -/// TabMail/Services/AppDatabase.swift → 20 +/// TabMail/Services/AppDatabase.swift → 21 /// rg -o '"v([0-9]+)_[A-Za-z0-9_]+"' -r '$1' \ -/// TabMail/Services/AppDatabase.swift | sort -n -u | awk '$1>=68' | wc -l → 20 +/// TabMail/Services/AppDatabase.swift | sort -n -u | awk '$1>=68' | wc -l → 21 /// ``` /// Equal counts are the invariant: every migration from `v68` up runs /// `.immediate`, none below `v68` does, and `everyLiveForeignKeyCascades` below -/// checks the premise that licenses it. At the current top (`v87`) the two -/// counts are 20; at 87 registered migrations that leaves 67 still +/// checks the premise that licenses it. At the current top (`v88`) the two +/// counts are 21; at 88 registered migrations that leaves 67 still /// running the whole-database check (66 on the GRDB default plus `v2`, the only /// explicit `.deferred` left). /// diff --git a/TabMailTests/Infrastructure/FakeIMAPServer.swift b/TabMailTests/Infrastructure/FakeIMAPServer.swift index 7c571dec..cc715158 100644 --- a/TabMailTests/Infrastructure/FakeIMAPServer.swift +++ b/TabMailTests/Infrastructure/FakeIMAPServer.swift @@ -283,6 +283,12 @@ final class FakeIMAPServer: @unchecked Sendable { /// messages, the FETCH over a range covering all N returns fewer than N. /// Empty for every pre-existing test. var fetchRecordSuppressedByMailbox: [String: Set] = [:] + /// Numeric MIME sections whose server response ignores a requested + /// `` range and returns the whole section without the + /// mandatory origin marker. This intentionally nonconforming shape + /// validates that SwiftMail rejects, rather than silently accepts, an + /// unbounded literal for a bounded request. + var partialRangeIgnoredSections: Set = [] /// Invariant test layer (2026-07-16) — wrong-message wire oracle, /// deliverable 1. The rfc822 Message-ID(s) the CURRENT test's user /// intention(s) target, registered via `expectMutation(rfc822MessageId:)`. @@ -1003,6 +1009,10 @@ final class FakeIMAPServer: @unchecked Sendable { withState { $0.fetchRecordSuppressedByMailbox[mailbox] = uids } } + func ignorePartialRange(forSection section: String) { + withState { _ = $0.partialRangeIgnoredSections.insert(section) } + } + /// Test seam (T1.2b): make this mailbox's SELECT/EXAMINE omit the /// `* OK [UIDVALIDITY n]` untagged response entirely. /// @@ -2395,7 +2405,8 @@ final class FakeIMAPServer: @unchecked Sendable { flags: state.flagsByMailbox[mailbox] ?? [:], uidSuppressed: state.fetchUidSuppressedByMailbox[mailbox] ?? [], internalDateSuppressed: state.fetchInternalDateSuppressedByMailbox[mailbox] ?? [], - recordSuppressed: state.fetchRecordSuppressedByMailbox[mailbox] ?? [] + recordSuppressed: state.fetchRecordSuppressedByMailbox[mailbox] ?? [], + partialRangeIgnoredSections: state.partialRangeIgnoredSections ) } let matched = parseSequenceSet(seqStr, uidMode: uidMode, messages: snapshot.messages) @@ -2460,7 +2471,22 @@ final class FakeIMAPServer: @unchecked Sendable { "BODY[\(section)]", "BODY.PEEK[\(section)]" ] - if patterns.contains(where: { itemsStr.contains($0) }) { + if let partial = patterns.compactMap({ + partialRange(in: itemsStr, after: $0) + }).first { + if snapshot.partialRangeIgnoredSections.contains(section) { + let str = String(data: bytes, encoding: .utf8) ?? "" + fetchItems.append("BODY[\(section)] {\(bytes.count)}\r\n\(str)") + } else { + let start = min(partial.offset, bytes.count) + let end = min(start + partial.count, bytes.count) + let slice = bytes.subdata(in: start.. {\(slice.count)}\r\n\(str)" + ) + } + } else if patterns.contains(where: { itemsStr.contains($0) }) { let str = String(data: bytes, encoding: .utf8) ?? "" fetchItems.append("BODY[\(section)] {\(bytes.count)}\r\n\(str)") } @@ -2474,6 +2500,23 @@ final class FakeIMAPServer: @unchecked Sendable { return response } + /// Parse the request suffix in `BODY.PEEK[section]`. + /// The response echoes only `` per RFC 3501 §7.4.2. + private func partialRange( + in fetchItems: String, + after token: String + ) -> (offset: Int, count: Int)? { + guard let tokenRange = fetchItems.range(of: token) else { return nil } + let suffix = fetchItems[tokenRange.upperBound...] + guard suffix.first == "<", let close = suffix.firstIndex(of: ">") else { return nil } + let valueStart = suffix.index(after: suffix.startIndex) + let fields = suffix[valueStart..= 0, + let count = Int(fields[1]), count > 0 else { return nil } + return (offset, count) + } + private func parseSequenceSet(_ seqStr: String, uidMode: Bool, messages: [Message]) -> [Message] { var results: [Message] = [] for part in seqStr.split(separator: ",").map(String.init) { diff --git a/TabMailTests/Providers/IMAPChunkedBodyFetchTests.swift b/TabMailTests/Providers/IMAPChunkedBodyFetchTests.swift new file mode 100644 index 00000000..74fefaa8 --- /dev/null +++ b/TabMailTests/Providers/IMAPChunkedBodyFetchTests.swift @@ -0,0 +1,509 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ + +import Foundation +import SwiftMail +import Testing +@testable import TabMail + +@Suite("IMAP bounded MIME-part fetching", .serialized) +struct IMAPChunkedBodyFetchTests { + private func provider(for server: FakeIMAPServer) -> IMAPProvider { + IMAPProvider( + host: "127.0.0.1", port: server.port, + username: server.username, password: server.password, + smtpHost: "127.0.0.1", smtpPort: 587, useTLS: false + ) + } + + private func multipartMessage( + uid: Int = 501, + headerPaddingBytes: Int = 0 + ) -> ( + message: FakeIMAPServer.Message, + text: Data, + attachmentAdvertisedSize: Int + ) { + let text = Data(repeating: Character("a").asciiValue!, count: 1024 * 1024 + 17) + let attachmentSize = 34 * 1024 * 1024 + let paddingHeader = headerPaddingBytes > 0 + ? "X-Oversized-Padding: \(String(repeating: "h", count: headerPaddingBytes))\r\n" + : "" + let header = """ + From: Sender \r + To: Recipient \r + Subject: Bounded batch\r + Date: Thu, 02 Oct 2025 01:50:00 +0000\r + Message-ID: \r + Content-Type: multipart/mixed; boundary="bounded"\r + \(paddingHeader) + \r + + """ + let bodystructure = """ + (("TEXT" "PLAIN" ("CHARSET" "UTF-8") NIL NIL "7BIT" \(text.count) 1)("APPLICATION" "PDF" ("NAME" "large.pdf") NIL NIL "BASE64" \(attachmentSize) NIL ("ATTACHMENT" ("FILENAME" "large.pdf"))) "MIXED") + """ + let message = FakeIMAPServer.makeMultipartMessage( + uid: uid, + subject: "Bounded batch", + from: "Sender ", + to: "Recipient ", + date: "Thu, 02 Oct 2025 01:50:00 +0000", + internalDate: "02-Oct-2025 01:50:00 +0000", + messageID: "", + rawHeader: header, + fullMessage: Data(header.utf8), + bodystructure: bodystructure, + partBodies: ["1": text, "2": Data("not downloaded".utf8)] + ) + return (message, text, attachmentSize) + } + + private func oversizedMetadataMessage(uid: Int) -> FakeIMAPServer.Message { + let oversizedSubject = String( + repeating: "s", + count: IMAPFetchMapping.responseBufferLimit + 1024 + ) + return FakeIMAPServer.makeMultipartMessage( + uid: uid, + subject: oversizedSubject, + from: "Sender ", + to: "Recipient ", + date: "Thu, 02 Oct 2025 01:50:00 +0000", + internalDate: "02-Oct-2025 01:50:00 +0000", + messageID: "", + rawHeader: "Date: Thu, 02 Oct 2025 01:50:00 +0000\r\n\r\n", + fullMessage: Data("Synthetic fixture".utf8), + bodystructure: "(\"TEXT\" \"PLAIN\" (\"CHARSET\" \"UTF-8\") NIL NIL \"7BIT\" 1 1)", + partBodies: ["1": Data("x".utf8)] + ) + } + + @Test("Singleton batch metadata overflow is attributed as terminal without an RFC 822 id") + func singletonBatchMetadataOverflowIsTerminal() async throws { + let server = FakeIMAPServer(messages: [oversizedMetadataMessage(uid: 601)]) + try server.start() + defer { server.stop() } + + let provider = provider(for: server) + try await provider.connect() + defer { Task { try? await provider.disconnect() } } + + do { + _ = try await provider.fetchMessagesBatch(ids: ["601"], folder: "INBOX") + Issue.record("Oversized singleton metadata must not remain indefinitely retryable") + } catch ProviderError.bodyIndexingUnsupported( + let id, let observedUidValidity, let fetchedRfc822MessageId + ) { + #expect(id == "601") + #expect(observedUidValidity == 1) + #expect(fetchedRfc822MessageId == nil) + } catch { + Issue.record("Unexpected singleton metadata error: \(error)") + } + } + + @Test("Single-message metadata overflow is attributed as terminal without an RFC 822 id") + func singleFetchMetadataOverflowIsTerminal() async throws { + let server = FakeIMAPServer(messages: [oversizedMetadataMessage(uid: 602)]) + try server.start() + defer { server.stop() } + + let provider = provider(for: server) + try await provider.connect() + defer { Task { try? await provider.disconnect() } } + + do { + _ = try await provider.fetchMessage(id: "602", folder: "INBOX") + Issue.record("Oversized single-message metadata must not remain indefinitely retryable") + } catch ProviderError.bodyIndexingUnsupported( + let id, let observedUidValidity, let fetchedRfc822MessageId + ) { + #expect(id == "602") + #expect(observedUidValidity == 1) + #expect(fetchedRfc822MessageId == nil) + } catch { + Issue.record("Unexpected single-message metadata error: \(error)") + } + } + + @Test("Multi-UID metadata overflow remains unattributed and retryable") + func multiUIDMetadataOverflowIsNotMisattributed() async throws { + let ordinary = multipartMessage(uid: 604).message + let server = FakeIMAPServer(messages: [ + oversizedMetadataMessage(uid: 603), + ordinary, + ]) + try server.start() + defer { server.stop() } + + let provider = provider(for: server) + try await provider.connect() + defer { Task { try? await provider.disconnect() } } + + do { + _ = try await provider.fetchMessagesBatch(ids: ["603", "604"], folder: "INBOX") + Issue.record("The synthetic multi-UID metadata response should exceed the parser limit") + } catch ProviderError.bodyIndexingUnsupported { + Issue.record("A multi-UID metadata overflow cannot be attributed to one member") + } catch { + #expect(String(describing: error).contains("PayloadTooLargeError")) + } + } + + @Test("Body metadata fetch omits an oversized raw header before chunking") + func bodyMetadataDoesNotFetchRawHeader() async throws { + let fixture = multipartMessage( + headerPaddingBytes: IMAPFetchMapping.responseBufferLimit + 1024 + ) + let server = FakeIMAPServer(messages: [fixture.message]) + try server.start() + defer { server.stop() } + + let provider = provider(for: server) + try await provider.connect() + defer { Task { try? await provider.disconnect() } } + + let fetched = try await provider.fetchMessagesBatch(ids: ["501"], folder: "INBOX") + #expect(fetched["501"]?.textBody?.utf8.count == fixture.text.count) + + let single = try await provider.fetchMessage(id: "501", folder: "INBOX") + #expect(single.textBody?.utf8.count == fixture.text.count) + + let onDemand = try await provider.fetchAttachment( + messageId: "501", + folder: "INBOX", + section: "1", + encoding: nil, + expectedObservedUidValidity: 1, + expectedRfc822MessageId: "bounded-batch@example.com" + ) + #expect(onDemand == fixture.text) + + let commands = server.recordedCommands() + #expect(commands.contains { $0.contains("BODYSTRUCTURE") }) + #expect(!commands.contains { command in + command.contains("BODY.PEEK[HEADER]") || command.contains("BODY[HEADER]") + }) + #expect(IMAPFetchMapping.bodyFetchMetadataOptions.contains(.envelope)) + #expect(IMAPFetchMapping.bodyFetchMetadataOptions.contains(.internalDate)) + #expect(IMAPFetchMapping.bodyFetchMetadataOptions.contains(.flags)) + #expect(IMAPFetchMapping.bodyFetchMetadataOptions.contains(.bodyStructure)) + #expect(!IMAPFetchMapping.bodyFetchMetadataOptions.contains(.fullHeader)) + } + + @Test("Background selection downloads render ingredients but not normal attachments") + func selectsOnlyRenderIngredients() { + let parts = [ + MessagePart(sectionString: "1", contentType: "text/plain; charset=utf-8"), + MessagePart(sectionString: "2", contentType: "text/html", disposition: "attachment", filename: "body.html"), + MessagePart(sectionString: "3", contentType: "text/calendar", disposition: "attachment", filename: "invite.ics"), + MessagePart(sectionString: "4", contentType: "image/png", disposition: "inline", contentId: "logo@example.com"), + MessagePart(sectionString: "5", contentType: "image/jpeg", disposition: "attachment", filename: "photo.jpg", contentId: "photo@example.com"), + MessagePart(sectionString: "6", contentType: "application/pdf", disposition: "attachment", filename: "report.pdf"), + MessagePart(sectionString: "7", contentType: "message/rfc822", disposition: "attachment", filename: "forwarded.eml"), + MessagePart(sectionString: "8", contentType: "text/plain", filename: "notes.txt"), + MessagePart(sectionString: "9", contentType: "text/plain", disposition: "inline", filename: "visible.txt"), + MessagePart(sectionString: "10", contentType: "message/rfc822", disposition: "attachment", filename: "forward.eml"), + MessagePart(sectionString: "10.1", contentType: "text/plain"), + MessagePart(sectionString: "10.2", contentType: "image/png", disposition: "inline", contentId: "nested@example.com"), + MessagePart(sectionString: "11", contentType: "message/rfc822", disposition: "inline", filename: "inline.eml"), + MessagePart(sectionString: "11.1", contentType: "text/plain"), + ] + + let selected = IMAPFetchMapping.requiredBodyPartIndices(in: parts) + .map { parts[$0].section.description } + #expect(selected == ["1", "3", "4", "9", "11.1"]) + } + + @Test("Encoded bytes are concatenated before one transfer-decoding pass") + func concatenatesBeforeDecoding() async throws { + let decoded = Data("chunk boundaries must not corrupt base64".utf8) + let encoded = decoded.base64EncodedData() + var requests: [(offset: Int, count: Int)] = [] + + let assembled = try await IMAPFetchMapping.concatenateEncodedPart( + expectedSize: encoded.count, + chunkSize: 5 + ) { offset, count in + requests.append((offset, count)) + let end = min(offset + count, encoded.count) + return encoded.subdata(in: offset..32 MiB attachment") + func providerBatchWireContract() async throws { + let fixture = multipartMessage() + let server = FakeIMAPServer(messages: [fixture.message]) + try server.start() + defer { server.stop() } + let provider = provider(for: server) + try await provider.connect() + defer { Task { try? await provider.disconnect() } } + + let result = try await provider.fetchMessagesBatch(ids: ["501"], folder: "INBOX") + let fetched = try #require(result["501"]) + #expect(fetched.observedUidValidity == 1) + #expect(fetched.textBody?.utf8.count == fixture.text.count) + #expect(fetched.attachments.first?.size == fixture.attachmentAdvertisedSize) + + let commands = server.recordedCommands() + #expect(commands.contains { $0.contains("BODY.PEEK[1]<0.1048576>") }) + #expect(commands.contains { $0.contains("BODY.PEEK[1]<1048576.17>") }) + #expect(commands.contains { $0.contains("BODY.PEEK[1]<1048593.1>") }) + #expect(!commands.contains { $0.contains("BODY.PEEK[2]") || $0.contains("BODY[2]") }) + } + + @Test("Ignored range is terminal but a transient NO remains retryable") + func providerClassifiesWireFailures() async throws { + let ignoredFixture = multipartMessage(uid: 502) + let ignoredServer = FakeIMAPServer(messages: [ignoredFixture.message]) + ignoredServer.ignorePartialRange(forSection: "1") + try ignoredServer.start() + defer { ignoredServer.stop() } + let ignoredProvider = provider(for: ignoredServer) + try await ignoredProvider.connect() + defer { Task { try? await ignoredProvider.disconnect() } } + + do { + _ = try await ignoredProvider.fetchMessagesBatch(ids: ["502"], folder: "INBOX") + Issue.record("An ignored partial range must not be accepted") + } catch ProviderError.bodyIndexingUnsupported( + let id, let observedUidValidity, let fetchedRfc822MessageId + ) { + #expect(id == "502") + #expect(observedUidValidity == 1) + #expect(fetchedRfc822MessageId == "bounded-batch@example.com") + } catch { + Issue.record("Unexpected ignored-range error: \(error)") + } + + let transientFixture = multipartMessage(uid: 503) + let transientServer = FakeIMAPServer(messages: [transientFixture.message]) + transientServer.failNextCommand(containing: "BODY.PEEK[1]") + try transientServer.start() + defer { transientServer.stop() } + let transientProvider = provider(for: transientServer) + try await transientProvider.connect() + defer { Task { try? await transientProvider.disconnect() } } + + do { + _ = try await transientProvider.fetchMessagesBatch(ids: ["503"], folder: "INBOX") + Issue.record("Injected NO should fail this attempt") + } catch ProviderError.bodyIndexingUnsupported { + Issue.record("A transient tagged NO must remain retryable") + } catch { + #expect(transientServer.consumedInjectedFailureCount() == 1) + #expect(!IMAPFetchMapping.isDeterministicPartialFetchFailure(error)) + #expect(String(describing: error).contains("Injected test failure")) + } + } +} diff --git a/TabMailTests/Providers/IMAPProviderMockNestedEmlTests.swift b/TabMailTests/Providers/IMAPProviderMockNestedEmlTests.swift index 48c75084..bda21218 100644 --- a/TabMailTests/Providers/IMAPProviderMockNestedEmlTests.swift +++ b/TabMailTests/Providers/IMAPProviderMockNestedEmlTests.swift @@ -19,8 +19,126 @@ import Foundation @Suite("IMAPProvider — nested rfc822 end-to-end", .serialized) struct IMAPProviderMockNestedEmlTests { + private actor SuspendedEmlFetch { + private var continuation: CheckedContinuation? + + func fetch() async -> Data { + await withCheckedContinuation { continuation = $0 } + } + + func waitUntilStarted() async { + while continuation == nil { await Task.yield() } + } + + func finish(with data: Data) { + continuation?.resume(returning: data) + continuation = nil + } + } + + private actor PresentationBarrier { + private var continuation: CheckedContinuation? + + func wait() async { + await withCheckedContinuation { continuation = $0 } + } + + func waitUntilBlocked() async { + while continuation == nil { await Task.yield() } + } + + func release() { + continuation?.resume() + continuation = nil + } + } + // MARK: - File-uploaded .eml (filename-based, octet-stream, base64 transfer) + @Test("A cancelled .eml fetch cannot freeze previews after its view disappears") + @MainActor + func cancelledEmlFetchDoesNotAcquirePreviewFreeze() async { + PreviewFreezeGate.shared.end() + defer { PreviewFreezeGate.shared.end() } + let suspendedFetch = SuspendedEmlFetch() + let attachment = AttachmentInfo( + filename: "attached.eml", + contentType: "message/rfc822", + section: "2", + size: 128, + encoding: nil + ) + let bytes = Data(""" + From: Sender \r + To: Recipient \r + Subject: Cancelled preview\r + \r + Body + """.utf8) + + let coordinator = EmlAttachmentPreviewTaskCoordinator() + let presentationTask = coordinator.start { + do { + _ = try await EmlAttachmentPreviewLoader.load(attachment: attachment) { + await suspendedFetch.fetch() + } + try EmlAttachmentPreviewLoader.beginPreviewFreeze() + } catch is CancellationError { + // The production task handles lifecycle cancellation silently. + } catch { + Issue.record("Unexpected load failure: \(error)") + } + } + await suspendedFetch.waitUntilStarted() + coordinator.cancel() + await suspendedFetch.finish(with: bytes) + await presentationTask.value + #expect(!PreviewFreezeGate.shared.isFrozen) + } + + @Test("Cancellation after .eml parsing still prevents preview freeze acquisition") + @MainActor + func cancellationBetweenLoadAndPresentationDoesNotAcquirePreviewFreeze() async { + PreviewFreezeGate.shared.end() + defer { PreviewFreezeGate.shared.end() } + let barrier = PresentationBarrier() + let attachment = AttachmentInfo( + filename: "attached.eml", + contentType: "message/rfc822", + section: "2", + size: 128, + encoding: nil + ) + let bytes = Data(""" + From: Sender \r + To: Recipient \r + Subject: Parsed before cancellation\r + \r + Body + """.utf8) + + let presentationTask = Task { @MainActor in + let payload = try await EmlAttachmentPreviewLoader.load(attachment: attachment) { + bytes + } + await barrier.wait() + try EmlAttachmentPreviewLoader.beginPreviewFreeze() + return payload + } + await barrier.waitUntilBlocked() + presentationTask.cancel() + await barrier.release() + + do { + _ = try await presentationTask.value + Issue.record("A cancelled presentation must not acquire the preview freeze") + } catch is CancellationError { + #expect(!PreviewFreezeGate.shared.isFrozen) + } catch { + Issue.record("Unexpected cancellation result: \(error)") + } + } + /// Regression test for the "messageNotFound" bug where tapping a nested /// attachment inside a file-uploaded `.eml` would throw because the /// recursive parent fetch carried `encoding: nil` instead of the outer @@ -31,7 +149,7 @@ struct IMAPProviderMockNestedEmlTests { /// `.eml` filename and `BASE64` transfer encoding. `BODY[
]` /// returns base64-encoded bytes that decode to a multipart RFC 822 /// containing a nested PDF. - @Test("fetchMessage + fetchAttachment round-trip for file-uploaded .eml with base64 transfer") + @Test("Background skips an opaque .eml; on-demand fetch preserves nested attachment access") func fileUploadedEmlBase64EndToEnd() async throws { let topHtml = "

TOP BODY

" let topHtmlBytes = Data(topHtml.utf8) @@ -147,38 +265,52 @@ struct IMAPProviderMockNestedEmlTests { try await provider.connect() defer { Task { try? await provider.disconnect() } } - // === Part 1: fetchMessage surfaces marker + nested PDF === + // === Part 1: background body fetch leaves the opaque .eml metadata-only === let info = try await provider.fetchMessage(id: "77", folder: "INBOX") + #expect(info.observedUidValidity == 1) let html = try #require(info.htmlBody) - #expect(html.contains("class=\"tm-eml-section\"")) - #expect(html.contains("data-filename=\"carrier.eml\"")) - #expect(html.contains("data-subject=\"IMAP NESTED SUBJECT\"")) - #expect(html.contains("NESTED BODY")) - - let nested = info.attachments.first { $0.filename == "imap-nested.pdf" } - let pdfAtt = try #require(nested) - #expect(pdfAtt.parentEmlSection == "2") - let expectedCompound = EmlParsing.nestedSection(parent: "2", index: 0) - #expect(pdfAtt.section == expectedCompound) - // The critical invariant: encoding is the PARENT's transfer encoding, - // not the nested attachment's. Tap-time dispatch uses this to - // base64-decode the parent correctly before EMLParser sees the bytes. - #expect(pdfAtt.encoding?.lowercased() == "base64") + #expect(html.contains("TOP BODY")) + #expect(!html.contains("tm-eml-section")) + #expect(!html.contains("NESTED BODY")) + #expect(info.attachments.allSatisfy { $0.filename != "imap-nested.pdf" }) + + let carrier = try #require(info.attachments.first { $0.filename == "carrier.eml" }) + #expect(carrier.section == "2") + #expect(carrier.encoding?.lowercased() == "base64") + + // === Part 2: tapping the .eml fetches and decodes its parent bytes === + + let preview = try await EmlAttachmentPreviewLoader.load(attachment: carrier) { + try await provider.fetchAttachment( + messageId: "77", folder: "INBOX", + section: carrier.section, encoding: carrier.encoding, + expectedObservedUidValidity: nil, + expectedRfc822MessageId: "outer@example.com" + ) + } + #expect(preview.html.contains("NESTED BODY")) + let nested = try #require( + preview.nestedAttachments.first { $0.filename == "imap-nested.pdf" } + ) - // === Part 2: fetchAttachment compound path — parent's base64 === - // === encoding is carried through the recursive call === + // === Part 3: an attachment selected inside that preview resolves through + // === the compound section and the same bounded parent-fetch path. === + let expectedCompound = EmlParsing.nestedSection(parent: "2", index: 0) let fetchedBytes = try await provider.fetchAttachment( messageId: "77", folder: "INBOX", - section: pdfAtt.section, encoding: pdfAtt.encoding + section: expectedCompound, encoding: carrier.encoding, + expectedObservedUidValidity: nil, + expectedRfc822MessageId: "outer@example.com" ) let fetchedString = String(data: fetchedBytes, encoding: .utf8) ?? "" + #expect(nested.filename == "imap-nested.pdf") #expect(fetchedString.contains("%%SYNTHETIC-IMAP-PDF%%")) } - @Test("fetchMessage emits marker for nested message/rfc822 part with BODYSTRUCTURE recursion") - func nestedRfc822EmitsMarker() async throws { + @Test("Background excludes attached message/rfc822 descendants but keeps parent metadata") + func nestedRfc822RemainsMetadataOnly() async throws { // Top-level body (section 1) — text/html. let topHtml = "

TOP BODY TEXT

" let topHtmlBytes = Data(topHtml.utf8) @@ -292,11 +424,137 @@ struct IMAPProviderMockNestedEmlTests { return } - // The whole point — htmlBody contains the top body AND the marker for - // the nested rfc822 part. + // The top-level render body is present, while the attached message and + // its flattened descendants remain metadata-only until explicitly opened. let html = try #require(info.htmlBody) #expect(html.contains("TOP BODY TEXT")) - #expect(html.contains("class=\"tm-eml-section\"")) - #expect(html.contains("INNER BODY TEXT")) + #expect(html.contains("tm-eml-section")) + #expect(!html.contains("INNER BODY TEXT")) + let attachedMessage = try #require(info.attachments.first { $0.filename == "inner.eml" }) + #expect(attachedMessage.section == "2") + + let preview = try await EmlAttachmentPreviewLoader.load(attachment: attachedMessage) { + try await provider.fetchAttachment( + messageId: "42", folder: "INBOX", + section: attachedMessage.section, encoding: attachedMessage.encoding, + expectedObservedUidValidity: 1, + expectedRfc822MessageId: "outer@example.com" + ) + } + #expect(preview.html.contains("INNER BODY TEXT")) + #expect(server.recordedCommands().contains { + $0.contains("BODY.PEEK[2]<0.\(innerRfc822Bytes.count)>") + }) + #expect(server.recordedCommands().contains { + $0.contains("BODY.PEEK[2]<\(innerRfc822Bytes.count).1>") + }) + } + + @Test("UIDVALIDITY turnover refuses normal and .eml attachment payloads") + func attachmentReadsRefuseUidTurnover() async throws { + let bodystructure = """ + (("TEXT" "PLAIN" ("CHARSET" "UTF-8") NIL NIL "7BIT" 4 1)("APPLICATION" "PDF" ("NAME" "document.pdf") NIL NIL "7BIT" 3 NIL ("attachment" ("filename" "document.pdf")))("APPLICATION" "OCTET-STREAM" ("NAME" "attached.eml") NIL NIL "7BIT" 8 NIL ("attachment" ("filename" "attached.eml"))) "MIXED") + """ + func message(id: String, normal: String, eml: String) -> FakeIMAPServer.Message { + FakeIMAPServer.makeMultipartMessage( + uid: 9, + subject: "Attachment identity", + from: "Sender ", + to: "Recipient ", + date: "Thu, 02 Oct 2025 01:50:00 +0000", + internalDate: "02-Oct-2025 01:50:00 +0000", + messageID: "<\(id)>", + rawHeader: "Message-ID: <\(id)>\r\nDate: Thu, 02 Oct 2025 01:50:00 +0000\r\n\r\n", + fullMessage: Data(), + bodystructure: bodystructure, + partBodies: [ + "1": Data("body".utf8), + "2": Data(normal.utf8), + "3": Data(eml.utf8), + ] + ) + } + + let originalId = "original-attachment@example.com" + let server = FakeIMAPServer(messages: [ + message(id: originalId, normal: "old", eml: "old-eml") + ]) + server.setUidValidity(41, for: "INBOX") + try server.start() + defer { server.stop() } + + let provider = IMAPProvider( + host: "127.0.0.1", port: server.port, + username: server.username, password: server.password, + smtpHost: "127.0.0.1", smtpPort: 587, useTLS: false + ) + try await provider.connect() + defer { Task { try? await provider.disconnect() } } + + // The row still names epoch 41/UID 9, while UID 9 now belongs to a + // different message in epoch 42. Neither a normal file nor a metadata- + // only .eml tap may read the replacement payload. + server.setMessages([ + message(id: "replacement@example.com", normal: "new", eml: "new-eml") + ], in: "INBOX") + server.setUidValidity(42, for: "INBOX") + + for section in ["2", "3"] { + do { + _ = try await provider.fetchAttachment( + messageId: "9", + folder: "INBOX", + section: section, + encoding: nil, + expectedObservedUidValidity: 41, + expectedRfc822MessageId: originalId + ) + Issue.record("Section \(section) was fetched across UIDVALIDITY turnover") + } catch ProviderError.uidValidityChanged(let folder, let stored, let live) { + #expect(folder == "INBOX") + #expect(stored == 41) + #expect(live == 42) + } catch { + Issue.record("Unexpected turnover refusal for section \(section): \(error)") + } + } + + // Epoch agreement alone must not override contradictory RFC identity. + // Refuse before requesting any normal attachment payload. + server.setUidValidity(41, for: "INBOX") + do { + _ = try await provider.fetchAttachment( + messageId: "9", + folder: "INBOX", + section: "2", + encoding: nil, + expectedObservedUidValidity: 41, + expectedRfc822MessageId: originalId + ) + Issue.record("Matching UIDVALIDITY accepted a contradictory Message-ID") + } catch ProviderError.actionIdentityResolutionFailed(let messageId) { + #expect(messageId == "9") + } catch { + Issue.record("Unexpected matching-epoch identity refusal: \(error)") + } + + do { + _ = try await provider.fetchAttachment( + messageId: "9", + folder: "INBOX", + section: "2", + encoding: nil, + expectedObservedUidValidity: nil, + expectedRfc822MessageId: originalId + ) + Issue.record("Message-ID fallback accepted a replacement message") + } catch ProviderError.actionIdentityResolutionFailed(let messageId) { + #expect(messageId == "9") + } catch { + Issue.record("Unexpected Message-ID fallback refusal: \(error)") + } + #expect(!server.recordedCommands().contains { command in + command.contains("BODY.PEEK[2]") || command.contains("BODY.PEEK[3]") + }) } } diff --git a/TabMailTests/Providers/IMAPSelectEpochMirrorTests.swift b/TabMailTests/Providers/IMAPSelectEpochMirrorTests.swift index 181850b7..b0f899eb 100644 --- a/TabMailTests/Providers/IMAPSelectEpochMirrorTests.swift +++ b/TabMailTests/Providers/IMAPSelectEpochMirrorTests.swift @@ -213,7 +213,9 @@ struct IMAPSelectEpochMirrorTests { // `try?` — see this test's doc comment. server.setUidValidity(Int(Self.e10), for: "INBOX") _ = try? await provider.fetchAttachment( - messageId: "1", folder: "INBOX", section: "1", encoding: nil) + messageId: "1", folder: "INBOX", section: "1", encoding: nil, + expectedObservedUidValidity: Int(Self.e10), + expectedRfc822MessageId: Self.rfc) #expect(provider.lastObservedUidValidity(folderPath: "INBOX") == Self.e10, "the attachment fetch's re-SELECT observed \(Self.e10) and the mirror must say so, not \(Self.e9)") } diff --git a/TabMailTests/Queues/OversizedBodyQuarantineTests.swift b/TabMailTests/Queues/OversizedBodyQuarantineTests.swift index f1d562dc..43a3428e 100644 --- a/TabMailTests/Queues/OversizedBodyQuarantineTests.swift +++ b/TabMailTests/Queues/OversizedBodyQuarantineTests.swift @@ -606,34 +606,13 @@ struct OversizedQuarantineResetReleaseTests { } } -// MARK: - The quarantine's UI consequence: wake lock vs. completion banner +// MARK: - The quarantine's UI consequence: truthful completion -/// Removing the illegal `bodyEmptyConfirmed = 1` stamp made the quarantined row stay -/// honestly incomplete, which is correct — and which means -/// `BackfillProgress.pendingBodyCount` (`headerComplete = 1 AND bodyComplete = 0 AND -/// bodyEmptyConfirmed = 0`) never reaches 0 for an account holding one oversized -/// message, so `BackfillProgress.isFullyComplete` is false forever for that account. -/// -/// `FastSyncView` had TWO consumers keyed off that single durable-completeness fact: -/// the "Sync Complete" banner AND `keepScreenAwake(while: !isAllComplete)`. The second -/// one is a battery-draining defect — the device screen was pinned awake indefinitely. -/// -/// The split asserted here: -/// - the WAKE LOCK is a question about the app's CURRENT activity, so it moved to -/// `FastSyncView.keepScreenAwakeWhileWorking` — the header walk plus the two body -/// queues' `isIdle`; -/// - the BANNER is a TRUTH CLAIM about the mailbox, so it stays on -/// `isFullyComplete`. Telling the user "Sync Complete" while a body is genuinely -/// missing would be a second defect, not a fix. -/// -/// The idle inputs here come from the REAL queue actors after a REAL quarantine, not -/// from hand-fed booleans, so these tests pin the causal chain -/// (quarantine → queue idle → lock released) rather than the predicate's arithmetic. -/// The suite is deliberately TWO-SIDED: a broken predicate that ALWAYS released would -/// satisfy the release cases alone, so every release case has a held counterpart driven -/// off a queue that still holds admitted work. -@Suite("Fast Sync keep-awake follows runnable queue state, not durable completeness") -struct FastSyncKeepAwakeTests { +/// Fast Sync now keeps the screen awake for the whole view lifetime, independent +/// of queue or durable-completion state. The completion banner remains a separate +/// truth claim and must still withhold "Sync Complete" for an unfinished body. +@Suite("Fast Sync completion banner remains truthful") +struct FastSyncCompletionBannerTests { /// A progress snapshot for an account whose header walk is done and whose ONLY /// remaining work is `pendingBodyCount` quarantined oversized bodies. Dates derive @@ -652,78 +631,6 @@ struct FastSyncKeepAwakeTests { return p } - @Test("A quarantined oversized message leaves both body queues idle, so the keep-awake lock is RELEASED even though pendingBodyCount is still non-zero") - func quarantinedOversizedReleasesTheWakeLock() async { - let active = ActiveBodyQueue() - let backfill = BackfillBodyQueue() - let activeOversized = activeItem("acc1:INBOX:1") - let backfillOversized = backfillItem("acc1:Archive:1") - #expect(await active.admit(activeOversized) == true) - #expect(await backfill.admit(backfillOversized) == true) - - // The real quarantine disposition — not a model of it. - await active.handlePayloadTooLarge(items: [activeOversized], folderPath: "INBOX") - await backfill.handlePayloadTooLarge(items: [backfillOversized], folderPath: "Archive") - - let activeIdle = await active.isIdle - let backfillIdle = await backfill.isIdle - #expect(activeIdle, "a quarantined item is removed from the queue, so the queue has no runnable work") - #expect(backfillIdle) - - // The account is genuinely NOT fully complete — the count still sees the row. - let progress = progressWithPendingBodies(1) - #expect(progress.pendingBodyCount == 1, "the quarantined row is still counted; the fix must not hide it") - #expect(!progress.isFullyComplete, "durable completeness is honestly withheld") - - // …and the wake lock is nonetheless released, because it no longer asks that - // question. This exact pairing IS the defect: pre-fix these two lines could not - // both hold. - #expect(FastSyncView.keepScreenAwakeWhileWorking( - accountHeadersDone: [progress.headersDone], - activeBodyIdle: activeIdle, - backfillBodyIdle: backfillIdle - ) == false, "an oversized-only remainder must not pin the screen awake") - } - - @Test("The keep-awake lock is HELD while the ACTIVE body queue still holds admitted work") - func heldWhileActiveQueueHasWork() async { - let active = ActiveBodyQueue() - #expect(await active.admit(activeItem("acc1:INBOX:2")) == true) - let activeIdle = await active.isIdle - #expect(activeIdle == false, "an admitted, un-quarantined item leaves the queue runnable") - - #expect(FastSyncView.keepScreenAwakeWhileWorking( - accountHeadersDone: [true], - activeBodyIdle: activeIdle, - backfillBodyIdle: true - ) == true, "the screen stays awake while bodies are actually being fetched") - } - - @Test("The keep-awake lock is HELD while the BACKFILL body queue still holds admitted work") - func heldWhileBackfillQueueHasWork() async { - let backfill = BackfillBodyQueue() - #expect(await backfill.admit(backfillItem("acc1:Archive:2")) == true) - let backfillIdle = await backfill.isIdle - #expect(backfillIdle == false) - - #expect(FastSyncView.keepScreenAwakeWhileWorking( - accountHeadersDone: [true], - activeBodyIdle: true, - backfillBodyIdle: backfillIdle - ) == true) - } - - @Test("The keep-awake lock is HELD while any account's header walk is unfinished, including an account with no progress entry yet") - func heldWhileAnyHeaderWalkUnfinished() { - // `FastSyncView.holdAwake` maps a MISSING progress entry to `false` - // (`…?.headersDone == true`), so a not-yet-reporting account holds the lock. - #expect(FastSyncView.keepScreenAwakeWhileWorking( - accountHeadersDone: [true, false], - activeBodyIdle: true, - backfillBodyIdle: true - ) == true) - } - @Test("DECISION: the Sync Complete banner stays gated on isFullyComplete — one quarantined oversized body withholds it rather than claiming a complete mailbox") func syncCompleteBannerStaysTruthful() { // Withheld while a body is genuinely missing… diff --git a/TabMailTests/Queues/TerminalUnindexedBodyTests.swift b/TabMailTests/Queues/TerminalUnindexedBodyTests.swift new file mode 100644 index 00000000..2a051859 --- /dev/null +++ b/TabMailTests/Queues/TerminalUnindexedBodyTests.swift @@ -0,0 +1,481 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ + +import Foundation +import GRDB +import Testing +@testable import TabMail + +@Suite("Terminal-unindexed body state", .serialized, .processGlobalState) +struct TerminalUnindexedBodyTests { + private func bodyHeader( + messageId: String, + folderId: String, + folderPath: String, + isInInbox: Bool, + subject: String + ) -> MessageHeader { + var header = MessageHeader( + messageId: messageId, + subject: subject, + from: "sender@example.com", + fromAddress: "sender@example.com", + to: "recipient@example.com", + date: Date(), + snippet: "", + folderId: folderId, + accountId: "terminal-account", + folderPath: folderPath, + isInInbox: isInInbox + ) + header.headerComplete = true + header.observedUidValidity = 7 + return header + } + + private func makeSwappedDatabase() throws -> ( + header: MessageHeader, + pool: DatabasePool, + restore: () -> Void + ) { + let directory = FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString) + try FileManager.default.createDirectory(at: directory, withIntermediateDirectories: true) + var configuration = Configuration() + configuration.foreignKeysEnabled = true + let pool = try DatabasePool( + path: directory.appendingPathComponent("terminal.sqlite").path, + configuration: configuration + ) + let appDatabase = try AppDatabase(dbPool: pool) + let previous = AppDatabase.shared.withLock { current -> AppDatabase? in + let old = current + current = appDatabase + return old + } + + var account = Account( + emailAddress: "terminal@example.com", + displayName: "Terminal", + provider: .imap + ) + account.id = "terminal-account" + var folder = Folder( + name: "Archive", + path: "Archive", + role: .archive, + accountId: account.id + ) + folder.lastKnownUidValidity = 7 + folder.backfillComplete = true + var header = MessageHeader( + messageId: "42", + subject: "Bounded fetch unsupported", + from: "sender@example.com", + fromAddress: "sender@example.com", + to: "recipient@example.com", + date: Date(), + snippet: "", + folderId: folder.id, + accountId: account.id, + folderPath: folder.path, + isInInbox: false + ) + header.headerComplete = true + header.observedUidValidity = folder.lastKnownUidValidity + try pool.write { db in + try account.insert(db) + try folder.insert(db) + try header.insert(db) + } + + let restore = { + AppDatabase.shared.withLock { $0 = previous } + TestDatabaseTeardown.retire(pool: pool, directory: directory) + } + return (header, pool, restore) + } + + @Test("Protocol refusal retires automatic work without claiming empty or indexed") + func terminalStateIsTruthfulAndExcludedFromQueues() async throws { + let fixture = try makeSwappedDatabase() + defer { fixture.restore() } + let header = fixture.header + let headerId = header.id + let pool = fixture.pool + let item = BodyFetchProcessor.Item( + headerId: header.id, + accountId: header.accountId, + folderPath: header.folderPath, + messageId: header.messageId, + isInInbox: false + ) + + #expect(await BodyFetchProcessor.markBodyUnindexed( + item: item, + reason: .partialFetchUnsupported, + observedUidValidity: 7, + fetchedRfc822MessageId: nil + )) + + let state = try await pool.read { db in + let row = try MessageHeader.fetchOne(db, key: headerId) + let candidates = try Int.fetchOne(db, sql: """ + SELECT COUNT(*) FROM messageHeader + WHERE headerComplete = 1 + AND bodyComplete = 0 + AND bodyEmptyConfirmed = 0 + AND bodyIndexingFailureReason IS NULL + """) ?? 0 + return (row, candidates) + } + let stored = try #require(state.0) + #expect(stored.bodyComplete == false) + #expect(stored.bodyEmptyConfirmed == false) + #expect(stored.bodyIndexingFailureReason + == BodyIndexingFailureReason.partialFetchUnsupported.rawValue) + #expect(state.1 == 0) + } + + @Test("Terminal rows stay excluded after queue restart and progress recomputation") + func terminalStateConvergesAcrossRestartAndProgress() async throws { + let fixture = try makeSwappedDatabase() + defer { fixture.restore() } + let header = fixture.header + let item = BodyFetchProcessor.Item( + headerId: header.id, + accountId: header.accountId, + folderPath: header.folderPath, + messageId: header.messageId, + isInInbox: false + ) + #expect(await BodyFetchProcessor.markBodyUnindexed( + item: item, + reason: .partialFetchUnsupported, + observedUidValidity: 7, + fetchedRfc822MessageId: nil + )) + + // Exercise both production restart selectors with the same terminal + // row in their respective populations. If either drops its terminal + // predicate, repopulation leaves storage non-empty immediately. + try await fixture.pool.write { database in + try database.execute( + sql: "UPDATE messageHeader SET isInInbox = 1 WHERE id = ?", + arguments: [header.id] + ) + } + let activeQueue = ActiveBodyQueue() + await activeQueue.repopulateFromDatabase() + #expect(await activeQueue.isIdle) + + try await fixture.pool.write { database in + try database.execute( + sql: "UPDATE messageHeader SET isInInbox = 0 WHERE id = ?", + arguments: [header.id] + ) + } + let backfillQueue = BackfillBodyQueue() + await backfillQueue.repopulateFromDatabase() + #expect(await backfillQueue.isIdle) + + let storedAccount = try await fixture.pool.read { database in + try Account.fetchOne(database, key: header.accountId) + } + let account = try #require(storedAccount) + let engine = SyncEngine() + await engine.updateBackfillProgressForAccount(account) + let progress = await MainActor.run { + AccountManagerState.shared.backfillProgressByAccount[header.accountId] + } + #expect(progress?.pendingBodyCount == 0) + #expect(progress?.unindexedBodyCount == 1) + #expect(progress?.isFullyComplete == true) + await MainActor.run { + AccountManagerState.shared.backfillProgressByAccount[header.accountId] = nil + } + } + + @Test("Smart Reindex clears the terminal reason for a fresh attempt") + func smartReindexRestoresRetryability() async throws { + let fixture = try makeSwappedDatabase() + defer { fixture.restore() } + let header = fixture.header + let headerId = header.id + let pool = fixture.pool + let item = BodyFetchProcessor.Item( + headerId: header.id, + accountId: header.accountId, + folderPath: header.folderPath, + messageId: header.messageId, + isInInbox: false + ) + #expect(await BodyFetchProcessor.markBodyUnindexed( + item: item, + reason: .partialFetchUnsupported, + observedUidValidity: 7, + fetchedRfc822MessageId: nil + )) + + let engine = SyncEngine() + await engine.resetCrawlState() + + let stored = try await pool.read { db in + try MessageHeader.fetchOne(db, key: headerId) + } + #expect(stored?.bodyIndexingFailureReason == nil) + #expect(stored?.bodyComplete == false) + #expect(stored?.bodyEmptyConfirmed == false) + } + + @Test("A stale failure cannot overwrite a concurrently completed body") + func completedBodyWinsTerminalizationRace() async throws { + let fixture = try makeSwappedDatabase() + defer { fixture.restore() } + let header = fixture.header + let headerId = header.id + let pool = fixture.pool + try await pool.write { db in + try db.execute( + sql: "UPDATE messageHeader SET bodyComplete = 1 WHERE id = ?", + arguments: [headerId] + ) + } + let item = BodyFetchProcessor.Item( + headerId: header.id, + accountId: header.accountId, + folderPath: header.folderPath, + messageId: header.messageId, + isInInbox: false + ) + + #expect(!(await BodyFetchProcessor.markBodyUnindexed( + item: item, + reason: .partialFetchUnsupported, + observedUidValidity: 7, + fetchedRfc822MessageId: nil + ))) + + let stored = try await pool.read { db in + try MessageHeader.fetchOne(db, key: headerId) + } + #expect(stored?.bodyComplete == true) + #expect(stored?.bodyIndexingFailureReason == nil) + } + + @Test("A failure observed after UIDVALIDITY turnover cannot retire the old row") + func epochTurnoverRefusesTerminalization() async throws { + let fixture = try makeSwappedDatabase() + defer { fixture.restore() } + let header = fixture.header + let item = BodyFetchProcessor.Item( + headerId: header.id, + accountId: header.accountId, + folderPath: header.folderPath, + messageId: header.messageId, + isInInbox: false + ) + + #expect(!(await BodyFetchProcessor.markBodyUnindexed( + item: item, + reason: .partialFetchUnsupported, + observedUidValidity: 8, + fetchedRfc822MessageId: nil + ))) + + let stored = try await fixture.pool.read { db in + try MessageHeader.fetchOne(db, key: header.id) + } + #expect(stored?.bodyIndexingFailureReason == nil) + } + + @Test("A matching Message-ID cannot override contradictory UIDVALIDITY evidence") + func epochContradictionOutranksMatchingMessageIdentity() async throws { + let fixture = try makeSwappedDatabase() + defer { fixture.restore() } + var updatedHeader = fixture.header + updatedHeader.rfc822MessageId = "stable@example.com" + let header = updatedHeader + try await fixture.pool.write { database in try header.update(database) } + let item = BodyFetchProcessor.Item( + headerId: header.id, + accountId: header.accountId, + folderPath: header.folderPath, + messageId: header.messageId, + isInInbox: false + ) + + #expect(!(await BodyFetchProcessor.markBodyUnindexed( + item: item, + reason: .partialFetchUnsupported, + observedUidValidity: 8, + fetchedRfc822MessageId: "stable@example.com" + ))) + + let stored = try await fixture.pool.read { database in + try MessageHeader.fetchOne(database, key: header.id) + } + #expect(stored?.bodyIndexingFailureReason == nil) + } + + @Test("Matching fetched Message-ID proves identity when SELECT omits UIDVALIDITY") + func messageIdentityCanProveTerminalization() async throws { + let fixture = try makeSwappedDatabase() + defer { fixture.restore() } + var updatedHeader = fixture.header + updatedHeader.rfc822MessageId = "" + let header = updatedHeader + try await fixture.pool.write { db in try header.update(db) } + let item = BodyFetchProcessor.Item( + headerId: header.id, + accountId: header.accountId, + folderPath: header.folderPath, + messageId: header.messageId, + isInInbox: false + ) + + #expect(await BodyFetchProcessor.markBodyUnindexed( + item: item, + reason: .partialFetchUnsupported, + observedUidValidity: nil, + fetchedRfc822MessageId: "stable@example.com" + )) + } + + @Test("Stuck diagnostics separate runnable and terminal bodyless rows") + func diagnosticsClassifyBodyStatesWithoutOverlap() async throws { + let fixture = try makeSwappedDatabase() + defer { fixture.restore() } + let id = fixture.header.id + + var counts = await StuckMessageDiagnostics.bodyStatusCounts(in: fixture.pool) + #expect(counts == .init(lockedEmpty: 0, failing: 0, pending: 1, terminalUnindexed: 0)) + + try await fixture.pool.write { db in + try db.execute( + sql: "UPDATE messageHeader SET emptyFetchCount = 2 WHERE id = ?", + arguments: [id] + ) + } + counts = await StuckMessageDiagnostics.bodyStatusCounts(in: fixture.pool) + #expect(counts == .init(lockedEmpty: 0, failing: 1, pending: 0, terminalUnindexed: 0)) + + try await fixture.pool.write { db in + try db.execute( + sql: """ + UPDATE messageHeader + SET emptyFetchCount = 0, bodyIndexingFailureReason = ? + WHERE id = ? + """, + arguments: [BodyIndexingFailureReason.partialFetchUnsupported.rawValue, id] + ) + } + counts = await StuckMessageDiagnostics.bodyStatusCounts(in: fixture.pool) + #expect(counts == .init(lockedEmpty: 0, failing: 0, pending: 0, terminalUnindexed: 1)) + #expect(counts.runnable == 0) + } + + @Test("Active inbox queue converges after a bounded-fetch refusal") + func activeQueueConvergesAndLeavesSiblingRetryable() async throws { + let fixture = try makeSwappedDatabase() + defer { fixture.restore() } + let target = bodyHeader( + messageId: "42", folderId: "terminal-account:INBOX", + folderPath: "INBOX", isInInbox: true, + subject: "Bounded fetch unsupported" + ) + let sibling = bodyHeader( + messageId: "43", folderId: "terminal-account:INBOX", + folderPath: "INBOX", isInInbox: true, + subject: "Retryable sibling" + ) + let originalHeaderId = fixture.header.id + try await fixture.pool.write { db in + var inbox = Folder( + name: "INBOX", path: "INBOX", role: .inbox, + accountId: target.accountId + ) + inbox.lastKnownUidValidity = 7 + try inbox.insert(db) + _ = try MessageHeader.deleteOne(db, key: originalHeaderId) + try target.insert(db) + try sibling.insert(db) + } + + let provider = MockEmailProvider(staleWindowMode: .uid) + await provider.setFetchMessageThrows(ProviderError.bodyIndexingUnsupported( + messageId: target.messageId, + observedUidValidity: 7, + fetchedRfc822MessageId: nil + )) + await AccountManager.shared.registerProviderForTesting( + accountId: target.accountId, provider: provider + ) + let queue = ActiveBodyQueue() + await queue.enqueueBatch([target, sibling]) + await queue.awaitDrain() + await AccountManager.shared.unregisterProviderForTesting(accountId: target.accountId) + + let state = try await fixture.pool.read { db in + ( + try MessageHeader.fetchOne(db, key: target.id), + try MessageHeader.fetchOne(db, key: sibling.id) + ) + } + #expect(state.0?.bodyIndexingFailureReason + == BodyIndexingFailureReason.partialFetchUnsupported.rawValue) + #expect(state.1?.bodyIndexingFailureReason == nil) + #expect(state.1?.bodyComplete == false) + #expect(await queue.isIdle) + } + + @Test("Backfill non-inbox queue converges after a bounded-fetch refusal") + func backfillQueueConvergesAndLeavesSiblingRetryable() async throws { + let fixture = try makeSwappedDatabase() + defer { fixture.restore() } + let target = fixture.header + let sibling = bodyHeader( + messageId: "43", folderId: target.folderId, + folderPath: target.folderPath, isInInbox: false, + subject: "Retryable sibling" + ) + try await fixture.pool.write { db in try sibling.insert(db) } + + let provider = MockEmailProvider(staleWindowMode: .uid) + await provider.setFetchMessageThrows(ProviderError.bodyIndexingUnsupported( + messageId: target.messageId, + observedUidValidity: 7, + fetchedRfc822MessageId: nil + )) + await AccountManager.shared.registerProviderForTesting( + accountId: target.accountId, provider: provider + ) + let queue = BackfillBodyQueue() + await queue.enqueue([ + .init( + headerId: target.id, accountId: target.accountId, + folderPath: target.folderPath, messageId: target.messageId, + isInInbox: false + ), + .init( + headerId: sibling.id, accountId: sibling.accountId, + folderPath: sibling.folderPath, messageId: sibling.messageId, + isInInbox: false + ), + ]) + await queue.awaitDrain() + await AccountManager.shared.unregisterProviderForTesting(accountId: target.accountId) + + let state = try await fixture.pool.read { db in + ( + try MessageHeader.fetchOne(db, key: target.id), + try MessageHeader.fetchOne(db, key: sibling.id) + ) + } + #expect(state.0?.bodyIndexingFailureReason + == BodyIndexingFailureReason.partialFetchUnsupported.rawValue) + #expect(state.1?.bodyIndexingFailureReason == nil) + #expect(state.1?.bodyComplete == false) + #expect(await queue.isIdle) + } +} diff --git a/TabMailTests/Services/BackfillProgressCompletionTests.swift b/TabMailTests/Services/BackfillProgressCompletionTests.swift index 11e72ef8..12302165 100644 --- a/TabMailTests/Services/BackfillProgressCompletionTests.swift +++ b/TabMailTests/Services/BackfillProgressCompletionTests.swift @@ -25,6 +25,7 @@ struct BackfillProgressCompletionTests { totalEmails: Int, ftsIndexed: Int, pendingBodyCount: Int, + unindexedBodyCount: Int = 0, uidTotal: Int = 0 ) -> BackfillProgress { var p = BackfillProgress( @@ -38,6 +39,7 @@ struct BackfillProgressCompletionTests { ) p.uidTotal = uidTotal p.pendingBodyCount = pendingBodyCount + p.unindexedBodyCount = unindexedBodyCount return p } @@ -80,6 +82,41 @@ struct BackfillProgressCompletionTests { #expect(p.isFullyComplete) } + @Test("Terminal-unindexed bodies allow truthful completion and a full progress bar") + func terminalUnindexedCompletesTruthfully() { + let p = progress( + headersDone: true, + totalEmails: 100, + ftsIndexed: 98, + pendingBodyCount: 0, + unindexedBodyCount: 2 + ) + #expect(p.isFullyComplete) + #expect(p.unindexedBodyCount == 2) + #expect(p.fractionComplete == 1.0) + #expect(BodyIndexingProgressText.completion(unindexedCount: 2) + == "Sync complete with 2 messages not indexed") + #expect(BodyIndexingProgressText.terminalCompletion( + isComplete: p.isFullyComplete, + unindexedCount: p.unindexedBodyCount + ) == "Sync complete with 2 messages not indexed") + } + + @Test("Terminal completion text handles singular and clean completion") + func terminalCompletionTextGrammar() { + #expect(BodyIndexingProgressText.completion(unindexedCount: 0) == "Sync complete") + #expect(BodyIndexingProgressText.completion(unindexedCount: 1) + == "Sync complete with 1 message not indexed") + #expect(BodyIndexingProgressText.terminalCompletion( + isComplete: false, + unindexedCount: 1 + ) == nil) + #expect(BodyIndexingProgressText.terminalCompletion( + isComplete: true, + unindexedCount: 0 + ) == nil) + } + @Test("Display fraction is unchanged (still ftsIndexed / totalEmails)") func fractionStillUsesDisplayCounts() { // Completion decoupled from the bar, but the bar still reflects indexed diff --git a/TabMailTests/Services/NSEDataBridgeTests.swift b/TabMailTests/Services/NSEDataBridgeTests.swift index 876f9724..830ef6ca 100644 --- a/TabMailTests/Services/NSEDataBridgeTests.swift +++ b/TabMailTests/Services/NSEDataBridgeTests.swift @@ -28,6 +28,27 @@ struct NSEDataBridgeTests { } } + @Test("Confirmed NSE body indexing clears an earlier terminal reason") + func confirmedBodyClearsTerminalReason() throws { + let db = try TestDatabase.make() + try TestDatabase.insertAccount(db) + try TestDatabase.insertFolder(db) + let header = try TestDatabase.insertMessageHeader(db, messageId: "nse-terminal-clear") + try db.write { connection in + try connection.execute( + sql: "UPDATE messageHeader SET bodyIndexingFailureReason = ? WHERE id = ?", + arguments: [BodyIndexingFailureReason.partialFetchUnsupported.rawValue, header.id] + ) + try NSEDataBridge.markConfirmedBodiesComplete([header.id], in: connection) + } + + let stored = try db.read { connection in + try MessageHeader.fetchOne(connection, key: header.id) + } + #expect(stored?.bodyComplete == true) + #expect(stored?.bodyIndexingFailureReason == nil) + } + // MARK: - Backend URL mirrors from BackendConfig toggle @Test("mirrorBackendConfig uses BackendConfig.apiBaseURL, not hardcoded URL") diff --git a/TabMailTests/Services/SyncMaintenanceTests.swift b/TabMailTests/Services/SyncMaintenanceTests.swift index 914fc8a0..4013ab1e 100644 --- a/TabMailTests/Services/SyncMaintenanceTests.swift +++ b/TabMailTests/Services/SyncMaintenanceTests.swift @@ -1402,14 +1402,29 @@ struct PlannerStatisticsRefreshTests { return Fixture(pool: pool, directory: directory, path: path) } - /// The row count `ANALYZE` last recorded for `table`, read out of `sqlite_stat1`. - /// `nil` when statistics have never been computed (the table does not exist yet) - /// or when they do not cover this table. + /// The row count `ANALYZE` last recorded for `table`. Prefer the table row or + /// a non-partial index; the migration-time empty-table fixture can contain + /// only a partial-index statistic, so fall back to it when no full statistic + /// exists. Once a real `ANALYZE` creates full statistics, their population is + /// the table count and must win over the intentionally smaller partial index. private func recordedRowCount(_ pool: DatabasePool, table: String) throws -> Int? { try pool.read { db in guard try db.tableExists("sqlite_stat1") else { return nil } let stat = try String.fetchOne( - db, sql: "SELECT stat FROM sqlite_stat1 WHERE tbl = ? LIMIT 1", arguments: [table]) + db, + sql: """ + SELECT s.stat + FROM sqlite_stat1 AS s + LEFT JOIN pragma_index_list(?) AS i ON i.name = s.idx + WHERE s.tbl = ? + ORDER BY CASE + WHEN s.idx IS NULL OR i.partial = 0 THEN 0 + ELSE 1 + END, s.idx + LIMIT 1 + """, + arguments: [table, table] + ) guard let leading = stat?.split(separator: " ").first else { return nil } return Int(leading) } diff --git a/TabMailTests/Services/UserLabelAccountIdentityTests.swift b/TabMailTests/Services/UserLabelAccountIdentityTests.swift index 480994a6..cdeaf585 100644 --- a/TabMailTests/Services/UserLabelAccountIdentityTests.swift +++ b/TabMailTests/Services/UserLabelAccountIdentityTests.swift @@ -343,8 +343,20 @@ struct UserLabelIdentityMigrationTests { folderId: "\(accountId):INBOX", accountId: accountId, folderPath: "INBOX", isInInbox: true) header.headerComplete = true - let toInsert = header - try db.write { try toInsert.insert($0) } + // Raw SQL because this fixture deliberately stops at v81 while the + // current MessageHeader model includes columns introduced later. + try db.write { connection in + try connection.execute(sql: """ + INSERT INTO messageHeader + (id, folderId, accountId, folderPath, isInInbox, messageId, + subject, `from`, fromAddress, `to`, date, snippet, headerComplete) + VALUES (?, ?, ?, ?, 1, ?, ?, ?, ?, ?, ?, ?, 1) + """, arguments: [ + header.id, header.folderId, header.accountId, header.folderPath, + header.messageId, header.subject, header.from, header.fromAddress, + header.to, header.date, header.snippet, + ]) + } return header } diff --git a/TabMailTests/Shared/IMAPFetchMappingTests.swift b/TabMailTests/Shared/IMAPFetchMappingTests.swift index 8e36d099..c75ed1b9 100644 --- a/TabMailTests/Shared/IMAPFetchMappingTests.swift +++ b/TabMailTests/Shared/IMAPFetchMappingTests.swift @@ -79,6 +79,20 @@ struct IMAPFetchMappingTests { #expect(!IMAPFetchMapping.hasTopLevelHTMLBodyPart(info: info)) } + @Test("hasTopLevelHTMLBodyPart: false for an attached HTML file") + func attachedHTMLIsNotDisplayBody() { + var info = makeInfo() + info.parts = [ + MessagePart( + sectionString: "1", + contentType: "text/html", + disposition: "attachment", + filename: "document.html" + ) + ] + #expect(!IMAPFetchMapping.hasTopLevelHTMLBodyPart(info: info)) + } + @Test("hasTopLevelHTMLBodyPart: false when html exists ONLY inside an attached .eml") func htmlOnlyNestedInRfc822IsNotTopLevel() { // A plain-text message with an attached .eml whose body is HTML. The HTML @@ -429,4 +443,52 @@ struct IMAPFetchMappingTests { let keywords = IMAPFetchMapping.customKeywords(from: info) #expect(keywords == ["tm_reply", "$Important"]) } + + @Test("Metadata-only CIDs do not consume the fetched inline-image cap") + func inlineImageCapAppliesAfterEligibilityAndDataFiltering() { + let message = makeMessage(parts: [ + MessagePart( + sectionString: "2.1", + contentType: "image/png", + disposition: "inline", + contentId: "nested@example.com" + ), + MessagePart( + sectionString: "3", + contentType: "image/png", + disposition: "inline", + contentId: "outer@example.com", + data: Data("outer".utf8) + ), + ]) + + let images = IMAPFetchMapping.extractInlineImages( + message: message, + maxInlineImages: 1, + eligibleSections: ["3"] + ) + #expect(images.count == 1) + #expect(images.first?.contentId == "outer@example.com") + #expect(images.first?.data == Data("outer".utf8)) + } + + @Test("A skipped nested calendar cannot hide a fetched top-level invite") + func calendarExtractionChoosesFirstFetchedEligiblePart() { + let topLevelICS = Data("BEGIN:VCALENDAR\r\nEND:VCALENDAR".utf8) + let message = makeMessage(parts: [ + MessagePart(sectionString: "2.1", contentType: "text/calendar", size: 900), + MessagePart( + sectionString: "3", + contentType: "text/calendar", + size: topLevelICS.count, + data: topLevelICS + ), + ]) + + #expect(IMAPFetchMapping.extractICSData( + message: message, + eligibleSections: ["3"] + ) == topLevelICS) + #expect(IMAPFetchMapping.extractICSData(message: message) == topLevelICS) + } } diff --git a/TabMailTests/ViewModels/OnDemandBodyFetchIntegrationTests.swift b/TabMailTests/ViewModels/OnDemandBodyFetchIntegrationTests.swift index fa2db5f9..99c92ca0 100644 --- a/TabMailTests/ViewModels/OnDemandBodyFetchIntegrationTests.swift +++ b/TabMailTests/ViewModels/OnDemandBodyFetchIntegrationTests.swift @@ -289,6 +289,66 @@ struct OnDemandBodyFetchErrorTests { // Verify the state is correct for poll to work #expect(vm.message != nil, "Message header should be set for poll to work") } + + @Test("A terminal row stops loadBody before any server fetch") + @MainActor + func terminalRowStopsInitialFetch() async throws { + let (pool, _) = try makeTestPool() + let header = try await insertFixtures(pool, messageId: "odf_terminal_initial_\(UUID().uuidString)") + try await pool.write { db in + try db.execute( + sql: "UPDATE messageHeader SET bodyIndexingFailureReason = ? WHERE id = ?", + arguments: [BodyIndexingFailureReason.partialFetchUnsupported.rawValue, header.id] + ) + } + var fetchBodyCalled = false + let vm = MessageDetailViewModel( + messageId: header.id, + dbPool: pool, + fetchBodyOverride: { _ in fetchBodyCalled = true } + ) + + await vm.loadBody() + + #expect(!fetchBodyCalled) + #expect(vm.messageBody == nil) + #expect(vm.isLoading == false) + #expect(vm.error == ProviderError.bodyIndexingUnsupported( + messageId: "", observedUidValidity: nil, fetchedRfc822MessageId: nil + ).localizedDescription) + } + + @Test("An open body poll stops when the row becomes terminal") + @MainActor + func terminalTransitionStopsPollBeforeRetry() async throws { + let (pool, _) = try makeTestPool() + let header = try await insertFixtures(pool, messageId: "odf_terminal_poll_\(UUID().uuidString)") + var fetchBodyCalled = false + let vm = MessageDetailViewModel( + messageId: header.id, + dbPool: pool, + fetchBodyOverride: { _ in fetchBodyCalled = true } + ) + vm._testSeedMessage(header) + vm.startBodyPoll() + try await Task.sleep(for: .milliseconds(100)) + try await pool.write { db in + try db.execute( + sql: "UPDATE messageHeader SET bodyIndexingFailureReason = ? WHERE id = ?", + arguments: [BodyIndexingFailureReason.partialFetchUnsupported.rawValue, header.id] + ) + } + + var waited = 0 + while vm.error == nil && waited < 30 { + try await Task.sleep(for: .milliseconds(100)) + waited += 1 + } + + #expect(vm.error != nil) + #expect(vm.isLoading == false) + #expect(!fetchBodyCalled) + } } // MARK: - Suite 4: Mark-read-on-open (independent of loadBody) diff --git a/project.yml b/project.yml index 7171c3d9..5b1d2d8a 100644 --- a/project.yml +++ b/project.yml @@ -35,7 +35,7 @@ settings: packages: SwiftMail: url: https://github.com/TabMail/SwiftMail.git - revision: a2d4a94f844db62843ef6aec16f3ed9462152acc + revision: 3a904d8a5257162cc3935ab0090bc94333dc8022 SwiftSoup: url: https://github.com/scinfu/SwiftSoup.git from: "2.6.0"