Repository navigation
Sync Inbox preferences offline - #367
Conversation
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 34 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughSummary
Privacy and security
Affected areas
Validation
WalkthroughThe PR adds offline Inbox preference synchronization, encrypted Product Sync storage, explicit field conflicts, Inbox settings controls, preference-driven thread rendering, and attachment metadata across mailbox providers. New sessions start in Unified Inbox. ChangesInbox preference synchronization
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai review |
|
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c71f36693a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/unwired-mail/unwired-mail/Features/Account/AccountView.swift`:
- Around line 1542-1546: Update the accountSettings routing for
openSettings(.inbox) to include InboxSettingsView, passing the existing
inboxPreferenceStore and navigationRequest values. Ensure the inbox case is
exposed through accountSettings so release builds can open and edit inbox
preferences.
In `@apps/unwired-mail/unwired-mail/Services/Mailbox/MailboxConnection.swift`:
- Around line 843-844: Populate hasAttachments during every mailbox metadata
conversion using each provider’s attachment metadata, and preserve the value
when reconstructing mailbox metadata so AccountView receives the correct state.
Update the relevant conversion and reconstruction methods around bccRecipients,
and add coverage for both attached and unattached messages.
In
`@apps/unwired-mail/unwired-mail/Services/ProductSync/InboxPreferenceStore.swift`:
- Around line 93-101: Update the restoration logic in the ProductSync preference
state-loading flow to use migration-aware decoding for older or partially
decodable local payloads, preserving recoverable stored preferences and
conflicts. If decoding cannot safely recover the stored state, mark restoration
as failed and prevent subsequent synchronization or lifecycle transitions from
persisting .empty or .defaults over it; add coverage for an older and partially
decodable payload.
- Around line 141-199: Bind synchronize() to the account generation captured at
startup, and after every await in its load/save retry flow exit if
updateSession() has advanced that generation. Ensure only the task belonging to
the current generation clears syncTask, while preserving cancellation handling
and actor isolation across account transitions. Add a blocking synchronization
test that switches accounts while loading and verifies no old-account state is
reconciled or saved into the new account.
In
`@apps/unwired-mail/unwired-mail/Services/ProductSync/InboxPreferenceSyncService.swift`:
- Around line 72-90: Update InboxPreferenceSyncService’s init(from:) to validate
the decoded schemaVersion against the client’s supported schema version and
reject newer versions before applying or reconciling any fields. Add a test
covering a future schema payload and verify decoding fails without allowing a
subsequent local write to discard unknown values.
In `@docs/adr/0019-sync-mail-workflow-preferences-not-device-state.md`:
- Around line 4-5: Update the ADR statement about transient navigation state to
explicitly identify the Mail View and selected mailbox as device-local state,
and state that every new session starts in Unified Inbox with Important
selected. Preserve the existing synchronization scope and use the precise
navigation terms from CONTEXT.md.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9ecc6cee-6b66-46f5-8dbb-75d45004262b
📒 Files selected for processing (14)
.changeset/sync-inbox-preferences.mdapps/unwired-mail/unwired-mail.xcodeproj/project.pbxprojapps/unwired-mail/unwired-mail/Features/Account/AccountView.swiftapps/unwired-mail/unwired-mail/Features/Settings/InboxSettingsView.swiftapps/unwired-mail/unwired-mail/Features/Settings/SettingsScene.swiftapps/unwired-mail/unwired-mail/Services/Mailbox/MailboxConnection.swiftapps/unwired-mail/unwired-mail/Services/ProductAccount/ProductAccountSession.swiftapps/unwired-mail/unwired-mail/Services/ProductSync/InboxPreferenceStore.swiftapps/unwired-mail/unwired-mail/Services/ProductSync/InboxPreferenceSyncService.swiftapps/unwired-mail/unwired-mailTests/InboxPreferenceSyncServiceTests.swiftapps/unwired-mail/unwired-mailTests/MailboxConnectionAdapterTests.swiftapps/unwired-mail/unwired-mailTests/SettingsDestinationRegistryTests.swiftdocs/adr/0019-sync-mail-workflow-preferences-not-device-state.mddocs/adr/0022-save-workflow-preferences-offline-with-explicit-conflicts.md
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: Apple Debug · Unwired Mail
- GitHub Check: Apple performance · Unwired Mail
🧰 Additional context used
📓 Path-based instructions (6)
.changeset/**/*
📄 CodeRabbit inference engine (AGENTS.md)
Create a Changeset for runtime behavior changes or exported type/API changes; tests-only, internal refactors, documentation-only changes, and JSDoc-only maintenance may omit one at maintainer discretion.
Files:
.changeset/sync-inbox-preferences.md
docs/adr/**/*
📄 CodeRabbit inference engine (AGENTS.md)
Record durable architecture decisions and privacy-boundary decisions in
docs/adr/.
Files:
docs/adr/0019-sync-mail-workflow-preferences-not-device-state.mddocs/adr/0022-save-workflow-preferences-offline-with-explicit-conflicts.md
**/*.md
⚙️ CodeRabbit configuration file
**/*.md: Treat docs as product truth, not marketing. Prefer precise domain language
from CONTEXT.md; flag undefined terms, synonym drift, and conflicts with
existing ADRs under docs/adr/. For ADR changes, require a clear decision,
consequences, and privacy or security boundary impact; flag silent
reversals of prior ADRs without an explicit supersession note. For
docs/agents/ and bootstrap docs, verify agent workflow, validation, and
triage guidance stay aligned with AGENTS.md. Flag broken cross-links,
speculative future claims presented as shipped behavior, and docs that
omit the Apple vs TypeScript ownership of a boundary they describe.
Files:
docs/adr/0019-sync-mail-workflow-preferences-not-device-state.mddocs/adr/0022-save-workflow-preferences-offline-with-explicit-conflicts.md
apps/unwired-mail/**/*.{swift,m,mm}
📄 CodeRabbit inference engine (AGENTS.md)
The Apple app must provide formatter, linter, and test commands. Run
zsh scripts/check-apple-lint.zshfor format and lint, and use the documentedxcodebuild testcommand for tests; SwiftLint runs in strict mode.
Files:
apps/unwired-mail/unwired-mailTests/MailboxConnectionAdapterTests.swiftapps/unwired-mail/unwired-mail/Services/ProductAccount/ProductAccountSession.swiftapps/unwired-mail/unwired-mail/Features/Settings/InboxSettingsView.swiftapps/unwired-mail/unwired-mailTests/SettingsDestinationRegistryTests.swiftapps/unwired-mail/unwired-mail/Features/Settings/SettingsScene.swiftapps/unwired-mail/unwired-mail/Services/Mailbox/MailboxConnection.swiftapps/unwired-mail/unwired-mail/Services/ProductSync/InboxPreferenceStore.swiftapps/unwired-mail/unwired-mail/Services/ProductSync/InboxPreferenceSyncService.swiftapps/unwired-mail/unwired-mailTests/InboxPreferenceSyncServiceTests.swiftapps/unwired-mail/unwired-mail/Features/Account/AccountView.swift
apps/unwired-mail/**/*.swift
📄 CodeRabbit inference engine (AGENTS.md)
Apple validation must use SwiftLint strict mode and Swift Format strict linting; if Apple tooling is unavailable, report that clearly in the final handoff.
Files:
apps/unwired-mail/unwired-mailTests/MailboxConnectionAdapterTests.swiftapps/unwired-mail/unwired-mail/Services/ProductAccount/ProductAccountSession.swiftapps/unwired-mail/unwired-mail/Features/Settings/InboxSettingsView.swiftapps/unwired-mail/unwired-mailTests/SettingsDestinationRegistryTests.swiftapps/unwired-mail/unwired-mail/Features/Settings/SettingsScene.swiftapps/unwired-mail/unwired-mail/Services/Mailbox/MailboxConnection.swiftapps/unwired-mail/unwired-mail/Services/ProductSync/InboxPreferenceStore.swiftapps/unwired-mail/unwired-mail/Services/ProductSync/InboxPreferenceSyncService.swiftapps/unwired-mail/unwired-mailTests/InboxPreferenceSyncServiceTests.swiftapps/unwired-mail/unwired-mail/Features/Account/AccountView.swift
⚙️ CodeRabbit configuration file
apps/unwired-mail/**/*.swift: Prioritize functional correctness, data protection, and Apple platform
behavior over style already enforced by swift-format and SwiftLint.
Check actor isolation, cancellation, lifecycle transitions, persistence,
retry and offline behavior, and Keychain accessibility. Flag credentials,
tokens, mail content, or decrypted Product Sync data that can be logged,
persisted outside approved device-local storage, or cross a documented
privacy boundary. Require focused tests for changed behavior and failure
paths.
Files:
apps/unwired-mail/unwired-mailTests/MailboxConnectionAdapterTests.swiftapps/unwired-mail/unwired-mail/Services/ProductAccount/ProductAccountSession.swiftapps/unwired-mail/unwired-mail/Features/Settings/InboxSettingsView.swiftapps/unwired-mail/unwired-mailTests/SettingsDestinationRegistryTests.swiftapps/unwired-mail/unwired-mail/Features/Settings/SettingsScene.swiftapps/unwired-mail/unwired-mail/Services/Mailbox/MailboxConnection.swiftapps/unwired-mail/unwired-mail/Services/ProductSync/InboxPreferenceStore.swiftapps/unwired-mail/unwired-mail/Services/ProductSync/InboxPreferenceSyncService.swiftapps/unwired-mail/unwired-mailTests/InboxPreferenceSyncServiceTests.swiftapps/unwired-mail/unwired-mail/Features/Account/AccountView.swift
**/*Tests/**/*.swift
⚙️ CodeRabbit configuration file
**/*Tests/**/*.swift: Verify externally observable behavior, privacy invariants, error paths,
cancellation or restart behavior where relevant, and deterministic
isolation. Flag fragile sleeps, ordering dependencies, and tests that
duplicate the implementation instead of exercising it.
Files:
apps/unwired-mail/unwired-mailTests/MailboxConnectionAdapterTests.swiftapps/unwired-mail/unwired-mailTests/SettingsDestinationRegistryTests.swiftapps/unwired-mail/unwired-mailTests/InboxPreferenceSyncServiceTests.swift
🪛 markdownlint-cli2 (0.23.2)
.changeset/sync-inbox-preferences.md
[warning] 5-5: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
🔇 Additional comments (12)
apps/unwired-mail/unwired-mail/Services/ProductSync/InboxPreferenceSyncService.swift (1)
3-61: LGTM!Also applies to: 93-283
apps/unwired-mail/unwired-mail/Services/ProductSync/InboxPreferenceStore.swift (1)
4-59: LGTM!Also applies to: 104-139, 202-262
apps/unwired-mail/unwired-mail/Services/ProductAccount/ProductAccountSession.swift (1)
138-142: LGTM!apps/unwired-mail/unwired-mailTests/InboxPreferenceSyncServiceTests.swift (1)
16-175: LGTM!Also applies to: 178-268
apps/unwired-mail/unwired-mailTests/SettingsDestinationRegistryTests.swift (1)
212-214: LGTM!Also applies to: 235-235, 938-942, 955-955
.changeset/sync-inbox-preferences.md (1)
1-5: LGTM!docs/adr/0022-save-workflow-preferences-offline-with-explicit-conflicts.md (1)
4-5: LGTM!apps/unwired-mail/unwired-mail/Features/Account/AccountView.swift (1)
1071-1121: LGTM!Also applies to: 1259-1259, 1448-1448, 1593-1603, 1652-1652, 2517-2517, 3224-3224, 3277-3279, 3522-3524, 3690-3813
apps/unwired-mail/unwired-mail/Features/Settings/InboxSettingsView.swift (1)
3-181: LGTM!apps/unwired-mail/unwired-mail/Features/Settings/SettingsScene.swift (1)
260-275: LGTM!Also applies to: 557-557, 1763-1821, 1894-1917
apps/unwired-mail/unwired-mail.xcodeproj/project.pbxproj (1)
84-87: LGTM!Also applies to: 203-206, 274-274, 322-323, 346-346, 500-500, 548-549, 570-570
apps/unwired-mail/unwired-mailTests/MailboxConnectionAdapterTests.swift (1)
4254-4256: LGTM!
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/unwired-mail/unwired-mailTests/InboxPreferenceSyncServiceTests.swift`:
- Around line 157-162: Replace the fixed 1,000-iteration Task.yield polling
around the second synchronization with an explicit completion signal, such as a
continuation tied to completion of the rescheduled save/synchronization. Await
that signal before the final assertions, while preserving the existing saveCount
and store-state expectations.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 89cdceb6-cb89-4db0-8feb-393f65beece2
📒 Files selected for processing (14)
apps/unwired-mail/unwired-mail/Features/Account/AccountView.swiftapps/unwired-mail/unwired-mail/Services/Gmail/GmailMessageMetadataService.swiftapps/unwired-mail/unwired-mail/Services/Mailbox/EWSMailboxConnectionAdapter.swiftapps/unwired-mail/unwired-mail/Services/Mailbox/IMAPMailboxConnectionAdapter.swiftapps/unwired-mail/unwired-mail/Services/Mailbox/MailboxConnection.swiftapps/unwired-mail/unwired-mail/Services/Mailbox/MicrosoftGraphMailboxConnectionAdapter.swiftapps/unwired-mail/unwired-mail/Services/Mailbox/SystemEWSClient.swiftapps/unwired-mail/unwired-mail/Services/Mailbox/SystemIMAPMailboxClient.swiftapps/unwired-mail/unwired-mail/Services/ProductSync/InboxPreferenceStore.swiftapps/unwired-mail/unwired-mailTests/EWSMailboxConnectionAdapterTests.swiftapps/unwired-mail/unwired-mailTests/GmailMessageMetadataServiceTests.swiftapps/unwired-mail/unwired-mailTests/IMAPMailboxConnectionAdapterTests.swiftapps/unwired-mail/unwired-mailTests/InboxPreferenceSyncServiceTests.swiftapps/unwired-mail/unwired-mailTests/MicrosoftGraphMailboxConnectionAdapterTests.swift
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: Apple performance · Unwired Mail
- GitHub Check: Apple Debug · Unwired Mail
🧰 Additional context used
📓 Path-based instructions (3)
apps/unwired-mail/**/*.{swift,m,mm}
📄 CodeRabbit inference engine (AGENTS.md)
The Apple app must provide formatter, linter, and test commands. Run
zsh scripts/check-apple-lint.zshfor format and lint, and use the documentedxcodebuild testcommand for tests; SwiftLint runs in strict mode.
Files:
apps/unwired-mail/unwired-mail/Services/Mailbox/IMAPMailboxConnectionAdapter.swiftapps/unwired-mail/unwired-mailTests/IMAPMailboxConnectionAdapterTests.swiftapps/unwired-mail/unwired-mail/Services/Mailbox/SystemEWSClient.swiftapps/unwired-mail/unwired-mail/Services/Mailbox/MailboxConnection.swiftapps/unwired-mail/unwired-mailTests/MicrosoftGraphMailboxConnectionAdapterTests.swiftapps/unwired-mail/unwired-mailTests/InboxPreferenceSyncServiceTests.swiftapps/unwired-mail/unwired-mail/Services/ProductSync/InboxPreferenceStore.swiftapps/unwired-mail/unwired-mail/Services/Mailbox/MicrosoftGraphMailboxConnectionAdapter.swiftapps/unwired-mail/unwired-mail/Services/Mailbox/SystemIMAPMailboxClient.swiftapps/unwired-mail/unwired-mail/Services/Gmail/GmailMessageMetadataService.swiftapps/unwired-mail/unwired-mailTests/GmailMessageMetadataServiceTests.swiftapps/unwired-mail/unwired-mail/Features/Account/AccountView.swiftapps/unwired-mail/unwired-mailTests/EWSMailboxConnectionAdapterTests.swiftapps/unwired-mail/unwired-mail/Services/Mailbox/EWSMailboxConnectionAdapter.swift
apps/unwired-mail/**/*.swift
📄 CodeRabbit inference engine (AGENTS.md)
Apple validation must use SwiftLint strict mode and Swift Format strict linting; if Apple tooling is unavailable, report that clearly in the final handoff.
Files:
apps/unwired-mail/unwired-mail/Services/Mailbox/IMAPMailboxConnectionAdapter.swiftapps/unwired-mail/unwired-mailTests/IMAPMailboxConnectionAdapterTests.swiftapps/unwired-mail/unwired-mail/Services/Mailbox/SystemEWSClient.swiftapps/unwired-mail/unwired-mail/Services/Mailbox/MailboxConnection.swiftapps/unwired-mail/unwired-mailTests/MicrosoftGraphMailboxConnectionAdapterTests.swiftapps/unwired-mail/unwired-mailTests/InboxPreferenceSyncServiceTests.swiftapps/unwired-mail/unwired-mail/Services/ProductSync/InboxPreferenceStore.swiftapps/unwired-mail/unwired-mail/Services/Mailbox/MicrosoftGraphMailboxConnectionAdapter.swiftapps/unwired-mail/unwired-mail/Services/Mailbox/SystemIMAPMailboxClient.swiftapps/unwired-mail/unwired-mail/Services/Gmail/GmailMessageMetadataService.swiftapps/unwired-mail/unwired-mailTests/GmailMessageMetadataServiceTests.swiftapps/unwired-mail/unwired-mail/Features/Account/AccountView.swiftapps/unwired-mail/unwired-mailTests/EWSMailboxConnectionAdapterTests.swiftapps/unwired-mail/unwired-mail/Services/Mailbox/EWSMailboxConnectionAdapter.swift
⚙️ CodeRabbit configuration file
apps/unwired-mail/**/*.swift: Prioritize functional correctness, data protection, and Apple platform
behavior over style already enforced by swift-format and SwiftLint.
Check actor isolation, cancellation, lifecycle transitions, persistence,
retry and offline behavior, and Keychain accessibility. Flag credentials,
tokens, mail content, or decrypted Product Sync data that can be logged,
persisted outside approved device-local storage, or cross a documented
privacy boundary. Require focused tests for changed behavior and failure
paths.
Files:
apps/unwired-mail/unwired-mail/Services/Mailbox/IMAPMailboxConnectionAdapter.swiftapps/unwired-mail/unwired-mailTests/IMAPMailboxConnectionAdapterTests.swiftapps/unwired-mail/unwired-mail/Services/Mailbox/SystemEWSClient.swiftapps/unwired-mail/unwired-mail/Services/Mailbox/MailboxConnection.swiftapps/unwired-mail/unwired-mailTests/MicrosoftGraphMailboxConnectionAdapterTests.swiftapps/unwired-mail/unwired-mailTests/InboxPreferenceSyncServiceTests.swiftapps/unwired-mail/unwired-mail/Services/ProductSync/InboxPreferenceStore.swiftapps/unwired-mail/unwired-mail/Services/Mailbox/MicrosoftGraphMailboxConnectionAdapter.swiftapps/unwired-mail/unwired-mail/Services/Mailbox/SystemIMAPMailboxClient.swiftapps/unwired-mail/unwired-mail/Services/Gmail/GmailMessageMetadataService.swiftapps/unwired-mail/unwired-mailTests/GmailMessageMetadataServiceTests.swiftapps/unwired-mail/unwired-mail/Features/Account/AccountView.swiftapps/unwired-mail/unwired-mailTests/EWSMailboxConnectionAdapterTests.swiftapps/unwired-mail/unwired-mail/Services/Mailbox/EWSMailboxConnectionAdapter.swift
**/*Tests/**/*.swift
⚙️ CodeRabbit configuration file
**/*Tests/**/*.swift: Verify externally observable behavior, privacy invariants, error paths,
cancellation or restart behavior where relevant, and deterministic
isolation. Flag fragile sleeps, ordering dependencies, and tests that
duplicate the implementation instead of exercising it.
Files:
apps/unwired-mail/unwired-mailTests/IMAPMailboxConnectionAdapterTests.swiftapps/unwired-mail/unwired-mailTests/MicrosoftGraphMailboxConnectionAdapterTests.swiftapps/unwired-mail/unwired-mailTests/InboxPreferenceSyncServiceTests.swiftapps/unwired-mail/unwired-mailTests/GmailMessageMetadataServiceTests.swiftapps/unwired-mail/unwired-mailTests/EWSMailboxConnectionAdapterTests.swift
🔇 Additional comments (21)
apps/unwired-mail/unwired-mail/Services/ProductSync/InboxPreferenceStore.swift (2)
268-275: Keep the replacement synchronization task registered.Line 272 clears
syncTaskafter an await without checking task or session ownership. If an account switch cancels this task and starts a replacement task, the obsolete task can clear the replacement handle. A later edit can then start concurrent synchronization.Bind the task to its session generation. Clear
syncTaskonly when that generation is still current.
73-73: LGTM!Also applies to: 204-204
apps/unwired-mail/unwired-mailTests/InboxPreferenceSyncServiceTests.swift (1)
225-225: LGTM!Also applies to: 242-242, 258-283
apps/unwired-mail/unwired-mail/Features/Account/AccountView.swift (1)
2034-2038: LGTM!apps/unwired-mail/unwired-mail/Services/Mailbox/MailboxConnection.swift (1)
844-844:hasAttachmentspropagation gap from the earlier review is resolved.
A prior review flagged thathasAttachmentswas left at its defaultfalseduring every provider-to-MailboxMessageMetadataconversion and during the reverse conversion back to provider metadata. This round addshasAttachments: hasAttachments ?? falseat Line 1088 (Gmail toMailboxMessageMetadata) andhasAttachments: hasAttachments ? true : nilat Line 1098 (MailboxMessageMetadataback to Gmail), and the equivalent EWS, IMAP, and Microsoft Graph adapters in this PR each populatehasAttachmentsin their ownmailboxMetadata()conversions. The gap is closed.Also applies to: 1071-1114
apps/unwired-mail/unwired-mail/Services/Gmail/GmailMessageMetadataService.swift (4)
13-13:hasAttachmentsdefault is consistent with the fetch logic.
This field staysniluntil Gmail explicitly reports an attachment, matching the ternary at Line 2667 that never assignsfalse. No issue here.
2631-2646: 🎯 Functional Correctness | 💤 Low valueVerify attachment detection at deep MIME nesting levels.
Thefieldsselector expandspartsto a fixed depth of four levels (payload(filename,headers,parts(filename,headers,parts(filename,headers,parts(filename,headers))))). Gmail message trees that nest multipart containers deeper than four levels (for example, forwarded messages wrapped in another multipart container, or S/MIME-wrapped messages) return a payload where deeper attachments are outside the requested field mask.GmailMessagePayload.hasAttachmentsat Line 3304 only sees the parts included in the response, so it reports no attachment even when one exists deeper in the tree.Confirm whether this depth is sufficient for the mailbox population you support, or add a fallback (for example, detect truncation and re-fetch with a wider field mask) for messages with deeper nesting.
3299-3306: 🎯 Functional Correctness | ⚡ Quick winVerify that
filenamedoes not also appear on inline (non-attachment) parts.
hasAttachmentstreats any part with a non-emptyfilenameas an attachment. Gmail API documentation states thatfilenameis present only for parts that represent an attachment, but some inline,cid:-referenced images observed in practice also carry afilenamevalue. If Gmail populatesfilenamefor inline images in your account population, this logic reportshasAttachments == truefor messages that only contain inline images, showing an incorrect attachment indicator to the user.Compare this against
IMAPBodyStructure.isAttachmentinSystemIMAPMailboxClient.swift, which additionally checks the Content-Disposition value. Confirm whether Gmail requires a similar check, for example by also inspecting the part'smimeTypeor aContent-Dispositionheader, before relying onfilenamealone.
2662-2686: LGTM!apps/unwired-mail/unwired-mail/Services/Mailbox/EWSMailboxConnectionAdapter.swift (2)
1571-1633: LGTM!
1682-1684:hasAttachmentsis now populated for EWS mailbox metadata.
This addresses the earlier finding thathasAttachmentswas not populated during mailbox metadata conversion. EWS reports a definitiveHasAttachmentsvalue inSystemEWSClient.swift(Line 956), sohasAttachments ?? falsehere correctly reflects the provider signal.apps/unwired-mail/unwired-mail/Services/Mailbox/IMAPMailboxConnectionAdapter.swift (1)
31-107: LGTM!apps/unwired-mail/unwired-mail/Services/Mailbox/MicrosoftGraphMailboxConnectionAdapter.swift (2)
367-481: LGTM!
1370-1424: LGTM!apps/unwired-mail/unwired-mail/Services/Mailbox/SystemEWSClient.swift (1)
413-413: LGTM!Also applies to: 956-956
apps/unwired-mail/unwired-mail/Services/Mailbox/SystemIMAPMailboxClient.swift (1)
778-839: Attachment detection follows the same disposition-aware heuristic aspreferredTextPart.
containsAttachmentreusesisAttachment, which checks the Content-Disposition list and theNAME/FILENAMEparameters. This is more precise than the Gmail path's filename-only check (see the corresponding comment onGmailMessageMetadataService.swift), and correctly excludes inline text parts that only carry aCHARSETparameter.apps/unwired-mail/unwired-mailTests/EWSMailboxConnectionAdapterTests.swift (1)
2895-2921: LGTM!Also applies to: 6119-6138
apps/unwired-mail/unwired-mailTests/GmailMessageMetadataServiceTests.swift (2)
943-985: LGTM!
6388-6656: LGTM!apps/unwired-mail/unwired-mailTests/IMAPMailboxConnectionAdapterTests.swift (1)
1119-1160: LGTM!apps/unwired-mail/unwired-mailTests/MicrosoftGraphMailboxConnectionAdapterTests.swift (1)
1738-1738: LGTM!Also applies to: 2082-2082, 2104-2104
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/adr/0019-sync-mail-workflow-preferences-not-device-state.md`:
- Line 5: Replace the term “Inbox presentation preferences” in the ADR
description with the canonical “Mail Workflow Preferences” terminology defined
by CONTEXT.md, while preserving the rest of the synchronization behavior and
details unchanged.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3a4b4269-5aa0-4ef7-ab21-dfd0de808868
📒 Files selected for processing (4)
apps/unwired-mail/unwired-mail/Services/ProductSync/InboxPreferenceStore.swiftapps/unwired-mail/unwired-mail/Services/ProductSync/InboxPreferenceSyncService.swiftapps/unwired-mail/unwired-mailTests/InboxPreferenceSyncServiceTests.swiftdocs/adr/0019-sync-mail-workflow-preferences-not-device-state.md
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: Apple performance · Unwired Mail
- GitHub Check: Apple Debug · Unwired Mail
🧰 Additional context used
📓 Path-based instructions (5)
docs/adr/**/*
📄 CodeRabbit inference engine (AGENTS.md)
Record durable architecture decisions and privacy-boundary decisions in
docs/adr/.
Files:
docs/adr/0019-sync-mail-workflow-preferences-not-device-state.md
**/*.md
⚙️ CodeRabbit configuration file
**/*.md: Treat docs as product truth, not marketing. Prefer precise domain language
from CONTEXT.md; flag undefined terms, synonym drift, and conflicts with
existing ADRs under docs/adr/. For ADR changes, require a clear decision,
consequences, and privacy or security boundary impact; flag silent
reversals of prior ADRs without an explicit supersession note. For
docs/agents/ and bootstrap docs, verify agent workflow, validation, and
triage guidance stay aligned with AGENTS.md. Flag broken cross-links,
speculative future claims presented as shipped behavior, and docs that
omit the Apple vs TypeScript ownership of a boundary they describe.
Files:
docs/adr/0019-sync-mail-workflow-preferences-not-device-state.md
apps/unwired-mail/**/*.{swift,m,mm}
📄 CodeRabbit inference engine (AGENTS.md)
The Apple app must provide formatter, linter, and test commands. Run
zsh scripts/check-apple-lint.zshfor format and lint, and use the documentedxcodebuild testcommand for tests; SwiftLint runs in strict mode.
Files:
apps/unwired-mail/unwired-mail/Services/ProductSync/InboxPreferenceSyncService.swiftapps/unwired-mail/unwired-mail/Services/ProductSync/InboxPreferenceStore.swiftapps/unwired-mail/unwired-mailTests/InboxPreferenceSyncServiceTests.swift
apps/unwired-mail/**/*.swift
📄 CodeRabbit inference engine (AGENTS.md)
Apple validation must use SwiftLint strict mode and Swift Format strict linting; if Apple tooling is unavailable, report that clearly in the final handoff.
Files:
apps/unwired-mail/unwired-mail/Services/ProductSync/InboxPreferenceSyncService.swiftapps/unwired-mail/unwired-mail/Services/ProductSync/InboxPreferenceStore.swiftapps/unwired-mail/unwired-mailTests/InboxPreferenceSyncServiceTests.swift
⚙️ CodeRabbit configuration file
apps/unwired-mail/**/*.swift: Prioritize functional correctness, data protection, and Apple platform
behavior over style already enforced by swift-format and SwiftLint.
Check actor isolation, cancellation, lifecycle transitions, persistence,
retry and offline behavior, and Keychain accessibility. Flag credentials,
tokens, mail content, or decrypted Product Sync data that can be logged,
persisted outside approved device-local storage, or cross a documented
privacy boundary. Require focused tests for changed behavior and failure
paths.
Files:
apps/unwired-mail/unwired-mail/Services/ProductSync/InboxPreferenceSyncService.swiftapps/unwired-mail/unwired-mail/Services/ProductSync/InboxPreferenceStore.swiftapps/unwired-mail/unwired-mailTests/InboxPreferenceSyncServiceTests.swift
**/*Tests/**/*.swift
⚙️ CodeRabbit configuration file
**/*Tests/**/*.swift: Verify externally observable behavior, privacy invariants, error paths,
cancellation or restart behavior where relevant, and deterministic
isolation. Flag fragile sleeps, ordering dependencies, and tests that
duplicate the implementation instead of exercising it.
Files:
apps/unwired-mail/unwired-mailTests/InboxPreferenceSyncServiceTests.swift
🔇 Additional comments (3)
apps/unwired-mail/unwired-mail/Services/ProductSync/InboxPreferenceSyncService.swift (1)
40-40: LGTM!Also applies to: 61-61, 75-83, 99-99
apps/unwired-mail/unwired-mail/Services/ProductSync/InboxPreferenceStore.swift (1)
28-58: LGTM!Also applies to: 106-108, 136-136, 185-267, 286-288, 327-327, 336-347
apps/unwired-mail/unwired-mailTests/InboxPreferenceSyncServiceTests.swift (1)
42-82: LGTM!Also applies to: 214-256, 298-313, 316-333, 356-382
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d30bc9b377
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
1 participant
Closes #117
Summary
The fixed Important Mail View remains owned by related issue #159; this change establishes the non-persisted Unified Inbox launch selection that view will use.
Validation
mise exec -- zsh scripts/check-apple-lint.zsh— passed; SwiftLint found 0 violations in 99 filesgit diff --check— passedplutil -lint apps/unwired-mail/unwired-mail.xcodeproj/project.pbxproj— passedxcodebuild testforInboxPreferenceSyncServiceTestsandSettingsDestinationRegistryTests— passed, 59 testsAll Simulator validation used iPhone 17 / iOS 26.5 UDID
D6D43A14-3B1A-41B6-B25A-0DFCA1BFB089after explicit boot readiness checks. No Busy recovery was required.Remaining human actions
None before merge or issue completion.
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.