Repository navigation
Rework the conversation reader around fixed actions - #421
Conversation
📝 WalkthroughSummary
WalkthroughThe conversation reader now uses a responsive contextual toolbar and exclusive message expansion. Category selection supports multiple searchable custom categories, creation, optimistic updates, cancellation, and bulk synchronization through mailbox metadata services. ChangesConversation reader and categories
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant MailShellConversationReader
participant GmailInboxViewModel
participant MailboxConnectionRouter
participant GmailMailboxConnectionAdapter
participant GmailMessageMetadataService
MailShellConversationReader->>GmailInboxViewModel: Apply selected category IDs
GmailInboxViewModel->>MailboxConnectionRouter: setCategories(categoryIds, message)
MailboxConnectionRouter->>GmailMailboxConnectionAdapter: Route by connection ID
GmailMailboxConnectionAdapter->>GmailMessageMetadataService: setCategories(categoryIds, message)
GmailMessageMetadataService-->>GmailMailboxConnectionAdapter: Return categorized metadata
GmailMailboxConnectionAdapter-->>GmailInboxViewModel: Return updated metadata
GmailInboxViewModel-->>MailShellConversationReader: Replace message category memberships
Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
gipity-bot
Bot
added
codex
codex-automation
human needed
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/unwired-mail/unwired-mail/Services/Gmail/GmailMessageMetadataService.swift (1)
2274-2288: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winUse exclusive locking for category writes.
GmailMailboxConnectionAdapter.setCategoriesuseswithSharedLock, whileGmailMessageMetadataService.setCategoriesperforms a full-list read-modify-write. Concurrent category assignments can load the same list and overwrite each other. UsewithLockor an atomic store operation.🤖 Prompt for 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. In `@apps/unwired-mail/unwired-mail/Services/Gmail/GmailMessageMetadataService.swift` around lines 2274 - 2288, The category update flow in GmailMessageMetadataService.setCategories must use exclusive locking for its full-list read-modify-write operation. Update the underlying GmailMailboxConnectionAdapter.setCategories path from withSharedLock to withLock, or use an equivalent atomic store operation, while preserving the existing category update behavior.
🤖 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 9076-9099: Update CustomCategoryEditorViewModel.create to use the
same reentrancy guard as setCategories before setting the shared isSaving flag,
so concurrent create/save operations cannot clear it prematurely. Preserve the
existing saveCategory, error handling, and defer-based reset behavior for the
guarded operation.
- Around line 8654-8670: Update replaceCategoryMemberships to preserve the
current MailboxMessageMetadata record and restore only its category identifiers
from the supplied message, rather than replacing the entire record. Ensure the
cancellation and failure calls using the pre-await message retain unrelated
in-flight changes such as read status while rolling back category membership.
- Around line 9816-9836: Update the category selector’s apply flow around
apply(selectedCategoryIds) to return or otherwise propagate the failure reason
from applyCategories instead of only a Boolean. Add sheet-local
applyErrorMessage state, set it when applying fails while preserving the
existing isApplying reset behavior, and attach an alert presenting that message
to the sheet’s inner NavigationStack so it appears above the presented sheet.
Add a focused test covering encrypted-sync failure and asserting the selector
displays the error.
- Around line 4448-4475: Update MessageCategorySelector to store a
MessageCategorySelection instance in state instead of its separate
selectedCategoryIds state. Replace the inline insert/remove logic with calls to
MessageCategorySelection.toggle(_:), and update reads and bindings to use the
model’s selectedCategoryIds so the UI routes selection changes through the
tested model.
- Around line 4845-4854: Update the .forward Button disabled condition to remove
the hasLoadedMessageBodyText(for:) cache requirement and gate only on
readerMutationIsDisabled and inboxViewModel.isLoadingMessageBody, allowing
prepareForward to fetch missing or evicted text. Add a focused test covering an
expanded message with evicted body text and assert that Forward remains enabled.
In `@apps/unwired-mail/unwired-mail/Services/Mailbox/MailboxConnection.swift`:
- Around line 865-871: Update MailboxMessageMetadata.assigningCategories and
GmailMessageMetadata.assigningCategories to use one shared category-ID
normalization helper, preserving empty strings while uniquing and sorting the
IDs. Remove the mailbox-specific filtering and ensure both implementations share
the same contract and normalization path.
- Around line 1372-1381: Remove the single-category default implementation from
MailboxMetadataSyncing.setCategories in
apps/unwired-mail/unwired-mail/Services/Mailbox/MailboxConnection.swift:1372-1381
so unsupported requests cannot report success without persistence. Apply the
same change to GmailMessageMetadataSyncing in
apps/unwired-mail/unwired-mail/Services/Gmail/GmailMessageMetadataService.swift:347-355.
In
apps/unwired-mail/unwired-mail/Services/Mailbox/IMAPMailboxConnectionAdapter.swift:2286-2293,
explicitly implement setCategories for IMAPMailboxConnectionAdapter,
MicrosoftGraphMailboxConnectionAdapter, and EWSMailboxConnectionAdapter,
supporting all category IDs or throwing
MailboxConnectionAdapterError.unsupportedProvider.
- Around line 2911-2931: Collapse the duplicate logic in
MailboxConnection.overrideCategory into setCategories, making overrideCategory
delegate through setCategories with the appropriate category input. Preserve
setCategories’ shared-lock acquisition, active-connection check, metadata
delegation, and connectionRemoved cleanup as the single implementation.
In `@apps/unwired-mail/unwired-mailTests/MailboxConnectionAdapterTests.swift`:
- Around line 9236-9245: Add a focused removal-cleanup test for
GmailMailboxConnectionAdapter.setCategories, modeled on
testGmailMailboxRemovalWaitsForInFlightCategoryOverride: start an in-flight
category update through DelayedAdapterProviderReadService, remove the
connection, release the gate, and assert the event log is ["categories-set",
"local-state-cleared"].
- Around line 5718-5741: Add reduced-capability coverage to
testConversationReaderToolbarKeepsAdaptiveActionOrder by invoking
MailShellReaderToolbarLayout.actions with canReplyAll false and an empty
providerActions list, while retaining the other capabilities enabled. Assert
compact returns [.reply, .forward, .category, .more] and regular returns
[.reply, .forward, .category, .pin, .more].
- Around line 1662-1709: Add focused tests alongside
testCategoryApplyUpdatesReaderMetadataBeforeEncryptedSyncCompletes for
GmailInboxViewModel.setCategories failure and cancellation paths. Make the
metadata service throw in one test, then assert the thread’s messageCategoryIds
revert to their original values and categoryOverrideErrorMessage is set; cancel
the applyTask in a second test and assert the categories revert without setting
an error message. Reuse the existing adapter and delayed-service setup patterns.
- Around line 1652-1659: Strengthen the test around assigningCategories by
passing an unsorted category list containing a duplicate, then assert that both
metadataService.setCategoryIds and updated.messageCategoryIds contain the
deduplicated, sorted category IDs. Preserve the existing adapter.setCategories
call and session setup while ensuring the assertions verify normalization.
- Around line 5706-5716: Add an assertion in the existing toggleMessageExpansion
test confirming the latest message remains expanded after attempting to toggle
it while expanded. Preserve the current assertions for olderMessage and use
isMessageExpanded(latestMessage, in: thread) to pin the non-collapsible
behavior.
---
Outside diff comments:
In
`@apps/unwired-mail/unwired-mail/Services/Gmail/GmailMessageMetadataService.swift`:
- Around line 2274-2288: The category update flow in
GmailMessageMetadataService.setCategories must use exclusive locking for its
full-list read-modify-write operation. Update the underlying
GmailMailboxConnectionAdapter.setCategories path from withSharedLock to
withLock, or use an equivalent atomic store operation, while preserving the
existing category update behavior.
🪄 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: 6e81c838-b3b4-460b-b1b5-a977b95bae06
📒 Files selected for processing (6)
.changeset/rework-conversation-reader.mdapps/unwired-mail/unwired-mail/Features/Account/AccountView.swiftapps/unwired-mail/unwired-mail/Services/Gmail/GmailMessageMetadataService.swiftapps/unwired-mail/unwired-mail/Services/Mailbox/IMAPMailboxConnectionAdapter.swiftapps/unwired-mail/unwired-mail/Services/Mailbox/MailboxConnection.swiftapps/unwired-mail/unwired-mailTests/MailboxConnectionAdapterTests.swift
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Apple performance · Unwired Mail
🧰 Additional context used
📓 Path-based instructions (4)
.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/rework-conversation-reader.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/Mailbox/IMAPMailboxConnectionAdapter.swiftapps/unwired-mail/unwired-mail/Services/Mailbox/MailboxConnection.swiftapps/unwired-mail/unwired-mail/Services/Gmail/GmailMessageMetadataService.swiftapps/unwired-mail/unwired-mailTests/MailboxConnectionAdapterTests.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-mail/Services/Mailbox/IMAPMailboxConnectionAdapter.swiftapps/unwired-mail/unwired-mail/Services/Mailbox/MailboxConnection.swiftapps/unwired-mail/unwired-mail/Services/Gmail/GmailMessageMetadataService.swiftapps/unwired-mail/unwired-mailTests/MailboxConnectionAdapterTests.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-mail/Services/Mailbox/IMAPMailboxConnectionAdapter.swiftapps/unwired-mail/unwired-mail/Services/Mailbox/MailboxConnection.swiftapps/unwired-mail/unwired-mail/Services/Gmail/GmailMessageMetadataService.swiftapps/unwired-mail/unwired-mailTests/MailboxConnectionAdapterTests.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.swift
🪛 markdownlint-cli2 (0.23.2)
.changeset/rework-conversation-reader.md
[warning] 4-4: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
🔇 Additional comments (19)
.changeset/rework-conversation-reader.md (1)
1-4: LGTM!apps/unwired-mail/unwired-mail/Features/Account/AccountView.swift (13)
1497-1499: LGTM!Also applies to: 1687-1688, 2177-2179
1541-1546: LGTM!
2937-2951: LGTM!Also applies to: 3021-3041
3203-3211: LGTM!
4408-4446: LGTM!
4500-4507: LGTM!Also applies to: 4665-4674, 4696-4697
4770-4844: LGTM!Also applies to: 4855-4899
4901-5002: LGTM!
8631-8653: LGTM!
8969-9075: LGTM!Also applies to: 9100-9174, 9207-9213
9680-9696: LGTM!Also applies to: 9751-9815, 9837-9916
5004-5028: 🗄️ Data Integrity & IntegrationNo reachable concurrent assignment exists. The reader sheet is the only production caller of
GmailInboxViewModel.setCategories, andMessageCategorySelectordisables Apply while the request runs.> Likely an incorrect or invalid review comment.
9084-9084: 🗄️ Data Integrity & IntegrationKeep the UUID identifier. Category IDs are opaque strings. Persistence, notification rules, and search support the UUID without a
custom:prefix.> Likely an incorrect or invalid review comment.apps/unwired-mail/unwired-mail/Services/Mailbox/MailboxConnection.swift (1)
843-844: LGTM!apps/unwired-mail/unwired-mail/Services/Gmail/GmailMessageMetadataService.swift (2)
331-336: LGTM!
2289-2314: LGTM!apps/unwired-mail/unwired-mailTests/MailboxConnectionAdapterTests.swift (2)
5743-5761: LGTM!
8989-8989: LGTM!Also applies to: 9117-9124
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/unwired-mail/unwired-mail/Features/Account/AccountView.swift (1)
4665-4674: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winFix: stale reader-level alert reappears after cancelling the category sheet.
MessageCategorySelectornow shows its own alert forapplyErrorMessage(Line 9851-9855), and the user dismisses it with "OK."applyCategories(Line 5013-5037) also setsreaderErrorMessage/readerErrorSource = .categoryOverrideon the same failure, onMailShellConversationReader.
applyCategoriesclearsreaderErrorMessage/readerErrorSourceonly on a later successful apply (Line 5031-5035). If the user dismisses the sheet's own error alert and then taps "Cancel" instead of retrying,categorySelectionbecomesniland the sheet closes, butreaderErrorMessageis still set.MailShellConversationReader's own.alert("Message action failed", ...)(Line 4675-4695) then becomes visible, re-showing the same error the user already acknowledged inside the sheet, under a different title.Clear the categoryOverride error state when the sheet dismisses, regardless of how it dismisses.
🐛 Proposed fix to clear stale reader-level error on sheet dismissal
- .sheet(item: $categorySelection) { selection in + .sheet(item: $categorySelection, onDismiss: { + if readerErrorSource == .categoryOverride { + readerErrorConnectionId = nil + readerErrorMessage = nil + readerErrorSource = nil + } + }) { selection in MessageCategorySelector( categoryChoices: categoryChoices, createCustomCategory: createCustomCategory, selection: selection, apply: { categoryIds in await applyCategories(categoryIds, to: selection.message) } ) }Add a focused test that fails the encrypted sync, dismisses the sheet's own alert, cancels the sheet, and asserts
readerErrorMessageno longer holds the acknowledged category-override error.As per path instructions: "Prioritize functional correctness, data protection, and Apple platform behavior over style already enforced by swift-format and SwiftLint" and "Require focused tests for changed behavior and failure paths."
Also applies to: 5013-5037, 9828-9863
🤖 Prompt for 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. In `@apps/unwired-mail/unwired-mail/Features/Account/AccountView.swift` around lines 4665 - 4674, Clear MailShellConversationReader’s readerErrorMessage and readerErrorSource when the categorySelection sheet dismisses, but only for the .categoryOverride source, so cancelling after acknowledging MessageCategorySelector’s applyErrorMessage cannot re-show the stale alert. Update the dismissal handling around categorySelection and add a focused test covering encrypted-sync failure, dismissing the selector alert, cancelling the sheet, and verifying readerErrorMessage is cleared.Source: Path instructions
🤖 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/Services/Mailbox/IMAPMailboxConnectionAdapter.swift`:
- Around line 1810-1820: Add focused failure-path tests for setCategories in
apps/unwired-mail/unwired-mail/Services/Mailbox/IMAPMailboxConnectionAdapter.swift:1810-1820,
apps/unwired-mail/unwired-mail/Services/Mailbox/EWSMailboxConnectionAdapter.swift:3569-3579,
and
apps/unwired-mail/unwired-mail/Services/Mailbox/MicrosoftGraphMailboxConnectionAdapter.swift:3202-3212,
using each adapter’s corresponding test suite. For each provider, call
setCategories with zero and multiple category IDs, assert
MailboxConnectionAdapterError.unsupportedProvider is thrown, and verify stored
message metadata remains unchanged.
---
Outside diff comments:
In `@apps/unwired-mail/unwired-mail/Features/Account/AccountView.swift`:
- Around line 4665-4674: Clear MailShellConversationReader’s readerErrorMessage
and readerErrorSource when the categorySelection sheet dismisses, but only for
the .categoryOverride source, so cancelling after acknowledging
MessageCategorySelector’s applyErrorMessage cannot re-show the stale alert.
Update the dismissal handling around categorySelection and add a focused test
covering encrypted-sync failure, dismissing the selector alert, cancelling the
sheet, and verifying readerErrorMessage is cleared.
🪄 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: 7dc70c7a-179a-4c1d-ab91-dda7c205c128
📒 Files selected for processing (12)
apps/unwired-mail/unwired-mail/Features/Account/AccountView.swiftapps/unwired-mail/unwired-mail/Services/Gmail/GmailMessageMetadataService.swiftapps/unwired-mail/unwired-mail/Services/Gmail/MessageCategorizationService.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-mailTests/GmailMessageMetadataServiceTests.swiftapps/unwired-mail/unwired-mailTests/GmailPushRelayServiceTests.swiftapps/unwired-mail/unwired-mailTests/IMAPMailboxConnectionAdapterTests.swiftapps/unwired-mail/unwired-mailTests/MailboxConnectionAdapterTests.swiftapps/unwired-mail/unwired-mailTests/PinSyncServiceTests.swift
💤 Files with no reviewable changes (1)
- apps/unwired-mail/unwired-mail/Services/Gmail/GmailMessageMetadataService.swift
📜 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 (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-mailTests/IMAPMailboxConnectionAdapterTests.swiftapps/unwired-mail/unwired-mail/Services/Mailbox/MicrosoftGraphMailboxConnectionAdapter.swiftapps/unwired-mail/unwired-mail/Services/Gmail/MessageCategorizationService.swiftapps/unwired-mail/unwired-mailTests/GmailPushRelayServiceTests.swiftapps/unwired-mail/unwired-mail/Services/Mailbox/IMAPMailboxConnectionAdapter.swiftapps/unwired-mail/unwired-mailTests/GmailMessageMetadataServiceTests.swiftapps/unwired-mail/unwired-mail/Services/Mailbox/EWSMailboxConnectionAdapter.swiftapps/unwired-mail/unwired-mailTests/PinSyncServiceTests.swiftapps/unwired-mail/unwired-mail/Services/Mailbox/MailboxConnection.swiftapps/unwired-mail/unwired-mailTests/MailboxConnectionAdapterTests.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/IMAPMailboxConnectionAdapterTests.swiftapps/unwired-mail/unwired-mail/Services/Mailbox/MicrosoftGraphMailboxConnectionAdapter.swiftapps/unwired-mail/unwired-mail/Services/Gmail/MessageCategorizationService.swiftapps/unwired-mail/unwired-mailTests/GmailPushRelayServiceTests.swiftapps/unwired-mail/unwired-mail/Services/Mailbox/IMAPMailboxConnectionAdapter.swiftapps/unwired-mail/unwired-mailTests/GmailMessageMetadataServiceTests.swiftapps/unwired-mail/unwired-mail/Services/Mailbox/EWSMailboxConnectionAdapter.swiftapps/unwired-mail/unwired-mailTests/PinSyncServiceTests.swiftapps/unwired-mail/unwired-mail/Services/Mailbox/MailboxConnection.swiftapps/unwired-mail/unwired-mailTests/MailboxConnectionAdapterTests.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/IMAPMailboxConnectionAdapterTests.swiftapps/unwired-mail/unwired-mail/Services/Mailbox/MicrosoftGraphMailboxConnectionAdapter.swiftapps/unwired-mail/unwired-mail/Services/Gmail/MessageCategorizationService.swiftapps/unwired-mail/unwired-mailTests/GmailPushRelayServiceTests.swiftapps/unwired-mail/unwired-mail/Services/Mailbox/IMAPMailboxConnectionAdapter.swiftapps/unwired-mail/unwired-mailTests/GmailMessageMetadataServiceTests.swiftapps/unwired-mail/unwired-mail/Services/Mailbox/EWSMailboxConnectionAdapter.swiftapps/unwired-mail/unwired-mailTests/PinSyncServiceTests.swiftapps/unwired-mail/unwired-mail/Services/Mailbox/MailboxConnection.swiftapps/unwired-mail/unwired-mailTests/MailboxConnectionAdapterTests.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/IMAPMailboxConnectionAdapterTests.swiftapps/unwired-mail/unwired-mailTests/GmailPushRelayServiceTests.swiftapps/unwired-mail/unwired-mailTests/GmailMessageMetadataServiceTests.swiftapps/unwired-mail/unwired-mailTests/PinSyncServiceTests.swiftapps/unwired-mail/unwired-mailTests/MailboxConnectionAdapterTests.swift
🔇 Additional comments (8)
apps/unwired-mail/unwired-mail/Features/Account/AccountView.swift (1)
4759-4765: LGTM!Also applies to: 8640-8685, 9090-9114, 9766-9944
apps/unwired-mail/unwired-mailTests/MailboxConnectionAdapterTests.swift (1)
1652-1663: LGTM!Also applies to: 1714-1797, 3625-3671, 5537-5550, 5830-5915, 5917-5936, 9319-9424
apps/unwired-mail/unwired-mail/Services/Mailbox/MailboxConnection.swift (1)
843-903: LGTM!Also applies to: 2884-2912
apps/unwired-mail/unwired-mail/Services/Gmail/MessageCategorizationService.swift (1)
1987-2013: LGTM!apps/unwired-mail/unwired-mailTests/GmailMessageMetadataServiceTests.swift (1)
7053-7062: LGTM!Also applies to: 7296-7309, 7568-7575, 7696-7703
apps/unwired-mail/unwired-mailTests/GmailPushRelayServiceTests.swift (1)
3807-3815: LGTM!apps/unwired-mail/unwired-mailTests/IMAPMailboxConnectionAdapterTests.swift (1)
1726-1733: LGTM!apps/unwired-mail/unwired-mailTests/PinSyncServiceTests.swift (1)
1147-1154: LGTM!
|
@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: d330dcebce
ℹ️ 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".
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/unwired-mail/unwired-mail/Features/Account/AccountView.swift (1)
9800-9824: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDisable category editing while Apply is in progress.
Line 9839 passes the current selection to
apply. The category rows andAdd New Categoryremain enabled while that operation awaits. A user can change the displayed selection after the request starts, but the earlier selection is the one that syncs and the sheet then dismisses.Disable the list content while
isApplyingis true. Add focused coverage for an apply operation that remains suspended.Proposed fix
.navigationTitle("Categories") .navigationBarTitleDisplayMode(.inline) .searchable(text: $query, prompt: "Search Categories") + .disabled(isApplying) .toolbar {Also applies to: 9835-9856
🤖 Prompt for 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. In `@apps/unwired-mail/unwired-mail/Features/Account/AccountView.swift` around lines 9800 - 9824, Update the category list content containing the `ForEach(filteredChoices)` rows and `Add New Category` button to be disabled while `isApplying` is true, preventing selection changes or category creation during `apply`. Add focused coverage for an apply operation that remains suspended and verify the list controls stay disabled until it completes.Sources: Coding guidelines, Path instructions
🤖 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.
Outside diff comments:
In `@apps/unwired-mail/unwired-mail/Features/Account/AccountView.swift`:
- Around line 9800-9824: Update the category list content containing the
`ForEach(filteredChoices)` rows and `Add New Category` button to be disabled
while `isApplying` is true, preventing selection changes or category creation
during `apply`. Add focused coverage for an apply operation that remains
suspended and verify the list controls stay disabled until it completes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: df9f6849-8162-4ff9-995e-5639e0f2fce1
📒 Files selected for processing (4)
apps/unwired-mail/unwired-mail/Features/Account/AccountView.swiftapps/unwired-mail/unwired-mail/Services/Mailbox/IMAPMailboxConnectionAdapter.swiftapps/unwired-mail/unwired-mailTests/IMAPMailboxConnectionAdapterTests.swiftapps/unwired-mail/unwired-mailTests/MailboxConnectionAdapterTests.swift
📜 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 (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/MailboxConnectionAdapterTests.swiftapps/unwired-mail/unwired-mailTests/IMAPMailboxConnectionAdapterTests.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-mail/Services/Mailbox/IMAPMailboxConnectionAdapter.swiftapps/unwired-mail/unwired-mailTests/MailboxConnectionAdapterTests.swiftapps/unwired-mail/unwired-mailTests/IMAPMailboxConnectionAdapterTests.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-mail/Services/Mailbox/IMAPMailboxConnectionAdapter.swiftapps/unwired-mail/unwired-mailTests/MailboxConnectionAdapterTests.swiftapps/unwired-mail/unwired-mailTests/IMAPMailboxConnectionAdapterTests.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/IMAPMailboxConnectionAdapterTests.swift
🔇 Additional comments (2)
apps/unwired-mail/unwired-mail/Features/Account/AccountView.swift (2)
9770-9791: LGTM!Also applies to: 9857-9893
4467-4469: 📐 Maintainability & Code QualityRun the required Apple validation.
Apple tooling and validation output were not available in this review context. Run
zsh scripts/check-apple-lint.zsh, then run the repository’s documentedxcodebuild testcommand after the compile fix.
apps/unwired-mail/unwired-mail/Features/Account/AccountView.swift#L4467-L4469: validate the Swift collection API fix with strict formatting and linting.apps/unwired-mail/unwired-mail/Services/Mailbox/IMAPMailboxConnectionAdapter.swift#L1437-L1464: validate the adapter categorization integration.apps/unwired-mail/unwired-mailTests/IMAPMailboxConnectionAdapterTests.swift#L442-L464: run the focused IMAP categorization tests.apps/unwired-mail/unwired-mailTests/MailboxConnectionAdapterTests.swift#L1631-L1663: run the focused category-assignment and rollback tests.Source: Coding guidelines
1 participant
Closes #160
Summary
Validation
pnpm install --frozen-lockfile— passedpnpm lint— passedpnpm format— passedpnpm turbo run check-types— passedpnpm test— passedpnpm fallow— passedmise exec -- zsh scripts/check-apple-lint.zsh— passed with zero violationstestEditingOutboxReplyOnSameConnectionPreservesProviderReplyMetadata(requestsReadReceipt:)failed deterministically for thetrueargument atMailboxConnectionAdapterTests.swift:6133becausereplacement.message.requestsReadReceiptwasfalse; the same failure reproduced in isolation and is outside this issue's reader/category scopeThe build also reports existing Swift 6 concurrency warnings in untouched product and test code.
Human actions
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.