Repository navigation
Revoke trusted devices securely - #257
Conversation
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 26 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 (3)
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughSummary
Affected areas
Validation
WalkthroughThis change implements trusted-device revocation and Product Sync key rotation. It adds backend state, key management, session revalidation, access fencing, settings controls, device-scoped synchronization, and related tests and documentation. ChangesTrusted-device revocation and Product Sync key rotation
Estimated code review effort: 5 (Critical) | ~120 minutes 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 |
|
@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: 99f21e7564
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 71f70e75aa
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8a5d032830
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 19
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/unwired-mail/unwired-mail/Services/Convex/ConvexClient.swift (1)
517-617: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winRemove the duplicated request path.
performRequestandperformNullableRequestare identical except for the final unwrap. The change adds a second verbatim copy of the application-failure mapping at Line 550 and Line 603. That mapping decides whether a revoked device is detected, andproductSyncKeyRotationuses the nullable path. If a future stable error code is added to one copy only, the two paths report different errors for the same backend response.Implement
performRequestin terms ofperformNullableRequest.♻️ Proposed refactor
private func performRequest<Response: Decodable>( endpoint: String, path: String, args: some Encodable, identityToken: String? ) async throws -> Response { - guard let convexURL else { - throw ConvexClientError.missingConvexURL - } - - var request = URLRequest(url: convexURL.appending(path: endpoint)) - // ... duplicated body ... - guard let value = functionResponse.value else { - throw ConvexClientError.decodeError - } - - return value + guard + let value: Response = try await performNullableRequest( + endpoint: endpoint, + path: path, + args: args, + identityToken: identityToken + ) + else { + throw ConvexClientError.decodeError + } + return value }🤖 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/Convex/ConvexClient.swift` around lines 517 - 617, Refactor performRequest to call performNullableRequest with the same endpoint, path, args, and identityToken, then unwrap and return the non-optional response value while preserving the existing decodeError behavior for nil. Remove the duplicated request-building, response validation, and Convex error-mapping logic from performRequest; keep performNullableRequest as the single implementation of the request path.apps/unwired-mail/unwired-mail/Services/Gmail/MessageCategorizationService.swift (1)
662-679: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftPreserve learning-signal payload identifiers across account-key rotation.
learningSignalPayloadIdentifierusesmaterial.accountKeyData, which changes duringrotatingAccountKey. After revocation, reads miss existing signals and writes create new payloads under new identifiers.decryptPayloadsupports legacy key versions only after a payload is found. Derive identifiers from a rotation-independent secret or migrate the old payloads, and add a rotation regression test.🤖 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/MessageCategorizationService.swift` around lines 662 - 679, Update the identifier derivation used by the learning-signal synchronization flow around learningSignalPayloadIdentifier so account-key rotation does not change existing payload identifiers. Use a rotation-independent secret, or migrate legacy payloads before reads and writes, while preserving decryptPayload’s legacy-version support; add a regression test covering reads and writes across rotatingAccountKey.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/Features/Settings/SettingsScene.swift`:
- Around line 996-1007: Update the error handling in the revoke flow around
service.revokeDevice so CancellationError is ignored without setting
errorMessage, matching the existing handling in presentRecoveryKey. Continue
assigning error.localizedDescription for all other errors.
- Around line 1449-1454: Add a device-specific accessibility-label helper named
revokeDevice(_:) beside renameDevice(_:), then apply it to the destructive
“Revoke” Button in the trusted-device list so VoiceOver identifies the
associated device while preserving the existing isWorking disabled state.
In
`@apps/unwired-mail/unwired-mail/Services/ProductAccount/ProductAccountService.swift`:
- Around line 524-526: Add a dedicated AccountAndDevicesServiceError case, such
as revocationUnavailable, with localized text stating that device revocation is
unavailable; then update revokeDevice’s rotationTransport guard to throw this
case when no transport is configured, leaving recoveryMaterialChanged for actual
recovery-material changes.
- Around line 509-516: Update the recoveryKeyStatus construction in
ProductAccountService to always call recoveryKeyStatus with the loaded local and
remote material, removing the rotationResponse non-nil fallback to .current.
After reconciliation, reload local material before this calculation so the
status reflects any recovery envelope replaced by reconcile and preserves the
existing healthy-rotation result.
- Around line 200-215: Extract the TRUSTED_DEVICE_REVOKED mapping from connect
into a shared ConvexProductAccountService error helper, then apply it to
productSyncRecoveryMaterial so ProductAccountServiceError.trustedDeviceRevoked
reaches the existing revoked-session cleanup path, including clearRevokedSession
and local credential cleanup. Preserve the current connect behavior while
reusing the shared translation.
In
`@apps/unwired-mail/unwired-mail/Services/ProductAccount/ProductAccountSession.swift`:
- Around line 110-120: Update ProductAccountSession.connect around the
productAccountId/trustedDeviceId guard to treat a changed trustedDeviceId with
the same productAccountId as re-registration after device removal, rather than
throwing differentAppleAccount or preserving the deleted ID; refresh the session
state using the new device ID and keep the existing different-account behavior
for productAccountId mismatches. Add a foreground revalidation test covering
this same-account trusted-device change.
In
`@apps/unwired-mail/unwired-mail/Services/ProductSync/ProductSyncEncryption.swift`:
- Around line 372-379: The three legacy key-map decoding sites in
ProductSyncEncryption.swift must avoid Dictionary(uniqueKeysWithValues:)
trapping when distinct string keys convert to the same Int version. At the
anchor site
apps/unwired-mail/unwired-mail/Services/ProductSync/ProductSyncEncryption.swift#L372-L379,
use collision-detecting dictionary construction and throw
ProductSyncEncryptionError.invalidKeyLength for duplicates, while renaming the
closure’s key binding so it does not shadow the keyVersion parameter; apply the
same throwing collision handling at the sibling sites `#L180-L187` and `#L253-L260`
so corrupted snapshots and recovery payloads remain recoverable errors.
- Around line 301-323: In rotatingAccountKey, qualify the archived existing key
and version with self by updating the legacyKeys assignment to use
self.accountKeyVersion and self.accountKeyData, preserving the parameter values
for the newly rotated key.
In `@apps/unwired-mail/unwired-mailTests/ProductAccountSessionTests.swift`:
- Around line 1901-1942: Add a test alongside
testForegroundRevalidationPurgesLocalDataAfterRemoteRevocation that uses a
product account service whose connect call throws a transient error only during
foreground revalidation after successful bootstrap. Assert revalidation
preserves the signed-in state, original snapshot in the session store, and
existing key material, while leaving valid session data intact.
In `@packages/convex/convex/productAccount.ts`:
- Around line 777-929: Split the revocation handler into named helpers for
completed-revocation replay, revocation during a pending rotation, fresh
rotation-material validation, and rotation start, keeping the top-level handler
focused on authentication, account/target lookup, and flow dispatch. Follow the
existing style of commitPendingProductSyncKeyRotation, and extract the repeated
keyEpoch/pendingDeviceCount/state response construction into one shared helper.
- Around line 827-835: Update the revocation handler around the
productSyncPendingKeyEpoch check so a target device that already adopted the
pending epoch can still be revoked. Remove the target from the trusted-device
and push-routing state immediately, then start the required follow-on Product
Sync epoch for remaining devices so revocation does not depend on pending
acknowledgements.
- Around line 875-880: Extract the recovery schema version literal from the
validation around nextKeyEpoch into a named constant representing the
Swift-client contract, then compare args.recoveryWrappedAccountKey.schemaVersion
against that constant while preserving the existing keyVersion and error
behavior.
- Around line 909-916: Remove the productSyncPendingRevokedDeviceId field from
the schema and every product account patch, including its initialization,
rotation assignment, and clear operation. Update the relevant product account
mutation flows while preserving all other product sync transition fields and
behavior.
In `@packages/convex/convex/productAccountAuth.ts`:
- Around line 47-62: Update requireRecentAuthentication to read iat from
UserIdentity’s additional-claims index signature and permit a small bounded
future-clock leeway while retaining the maximum-age check. Verify Apple-issued
identities include iat, and update convex-test.withIdentity coverage to inject
and validate that claim so protected mutations continue to authenticate
correctly.
In `@packages/convex/convex/productSync.ts`:
- Around line 100-107: Extract the required key-epoch derivation and validation
into one exported helper near requireProductAccount in productAccountAuth.ts,
using a single exported initialProductSyncKeyEpoch constant from
productAccount.ts instead of the fallback literal 1. Replace the duplicated
logic in writePayload and writeEncryptedPayloadIfUnchanged with calls to that
helper, preserving pending-epoch precedence and the existing rejection behavior.
In `@packages/convex/convex/schema.ts`:
- Around line 72-88: Update the revokedTrustedDevices.trustedDeviceId field in
the schema to use v.id('trustedDevices') instead of v.string(), preserving the
branded ID type used by productAccount.ts and productAccountAuth.ts. Apply the
same type correction to productSyncPendingRevokedDeviceId, while keeping
tombstone references valid even when the target device is deleted.
- Around line 10-31: Update the schema definitions for
productSyncPendingEncryptedTransition and
productSyncPendingRecoveryWrappedAccountKey to reuse the shared
encryptedProductSyncPayloadBodyValidator from `@private-email/contracts` instead
of duplicating the inline v.object validators. Preserve both fields as optional
and leave the surrounding productSyncPending fields unchanged.
In `@packages/convex/test/productAccount.test.ts`:
- Around line 231-364: Add negative authorization assertions for
revokeTrustedDevice in the existing product-account test: invoke it with a stale
identity iat and expect “Recent authentication required,” use the current device
as both trustedDeviceId and trustedDeviceToRevokeId and expect “Use sign out to
remove the current Trusted Device,” and target a device belonging to another
Product Account and expect “Trusted device required.” Create any separate
account/device fixtures needed while preserving the existing success-path
assertions.
In `@packages/convex/test/productSync.test.ts`:
- Around line 496-518: Add a test near the existing removed-device case that
creates a second connected device/account, then have the authenticated asUser
query getEncryptedPayload and listEncryptedPayloads using
otherConnect.trustedDeviceId. Assert both requests reject with “Trusted device
required,” while preserving asUser’s own account setup.
---
Outside diff comments:
In `@apps/unwired-mail/unwired-mail/Services/Convex/ConvexClient.swift`:
- Around line 517-617: Refactor performRequest to call performNullableRequest
with the same endpoint, path, args, and identityToken, then unwrap and return
the non-optional response value while preserving the existing decodeError
behavior for nil. Remove the duplicated request-building, response validation,
and Convex error-mapping logic from performRequest; keep performNullableRequest
as the single implementation of the request path.
In
`@apps/unwired-mail/unwired-mail/Services/Gmail/MessageCategorizationService.swift`:
- Around line 662-679: Update the identifier derivation used by the
learning-signal synchronization flow around learningSignalPayloadIdentifier so
account-key rotation does not change existing payload identifiers. Use a
rotation-independent secret, or migrate legacy payloads before reads and writes,
while preserving decryptPayload’s legacy-version support; add a regression test
covering reads and writes across rotatingAccountKey.
🪄 Autofix (Beta)
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: 722b4d59-4211-43a2-9a28-7c5f170713ab
📒 Files selected for processing (31)
.changeset/rotate-trusted-device-keys.mdREADME.mdapps/unwired-mail/unwired-mail/Features/Account/AccountView.swiftapps/unwired-mail/unwired-mail/Features/Settings/SettingsScene.swiftapps/unwired-mail/unwired-mail/Services/Convex/ConvexClient.swiftapps/unwired-mail/unwired-mail/Services/Gmail/MessageCategorizationService.swiftapps/unwired-mail/unwired-mail/Services/ProductAccount/ProductAccountService.swiftapps/unwired-mail/unwired-mail/Services/ProductAccount/ProductAccountSession.swiftapps/unwired-mail/unwired-mail/Services/ProductSync/CustomCategorySyncService.swiftapps/unwired-mail/unwired-mail/Services/ProductSync/MailboxConnectionSyncModels.swiftapps/unwired-mail/unwired-mail/Services/ProductSync/MailboxConnectionSyncService.swiftapps/unwired-mail/unwired-mail/Services/ProductSync/NotificationRuleSyncService.swiftapps/unwired-mail/unwired-mail/Services/ProductSync/PinSyncService.swiftapps/unwired-mail/unwired-mail/Services/ProductSync/ProductSyncEncryption.swiftapps/unwired-mail/unwired-mailTests/ConvexClientTests.swiftapps/unwired-mail/unwired-mailTests/CustomCategorySyncServiceTests.swiftapps/unwired-mail/unwired-mailTests/GmailMessageMetadataServiceTests.swiftapps/unwired-mail/unwired-mailTests/MailboxConnectionSyncServiceTests.swiftapps/unwired-mail/unwired-mailTests/MessageCategorizationServiceTests.swiftapps/unwired-mail/unwired-mailTests/NotificationRuleSyncServiceTests.swiftapps/unwired-mail/unwired-mailTests/PinSyncServiceTests.swiftapps/unwired-mail/unwired-mailTests/ProductAccountSessionTests.swiftapps/unwired-mail/unwired-mailTests/ProductSyncEncryptionTests.swiftapps/unwired-mail/unwired-mailTests/ProductSyncKeyMaterialStoreTests.swiftdocs/adr/0020-revoke-devices-with-sync-key-rotation.mdpackages/convex/convex/productAccount.tspackages/convex/convex/productAccountAuth.tspackages/convex/convex/productSync.tspackages/convex/convex/schema.tspackages/convex/test/productAccount.test.tspackages/convex/test/productSync.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Apple · Unwired Mail
🧰 Additional context used
📓 Path-based instructions (8)
apps/unwired-mail/**/*.{swift,mm,m,h}
📄 CodeRabbit inference engine (AGENTS.md)
The Apple app must provide formatter, linter, and test commands; use
zsh scripts/check-apple-lint.zshfor formatting and linting and the specifiedxcodebuild testcommand for tests.
Files:
apps/unwired-mail/unwired-mail/Services/ProductSync/MailboxConnectionSyncService.swiftapps/unwired-mail/unwired-mailTests/PinSyncServiceTests.swiftapps/unwired-mail/unwired-mail/Services/ProductSync/NotificationRuleSyncService.swiftapps/unwired-mail/unwired-mail/Services/ProductSync/MailboxConnectionSyncModels.swiftapps/unwired-mail/unwired-mail/Services/ProductSync/PinSyncService.swiftapps/unwired-mail/unwired-mailTests/GmailMessageMetadataServiceTests.swiftapps/unwired-mail/unwired-mail/Services/ProductSync/CustomCategorySyncService.swiftapps/unwired-mail/unwired-mailTests/CustomCategorySyncServiceTests.swiftapps/unwired-mail/unwired-mail/Services/Gmail/MessageCategorizationService.swiftapps/unwired-mail/unwired-mailTests/NotificationRuleSyncServiceTests.swiftapps/unwired-mail/unwired-mailTests/MailboxConnectionSyncServiceTests.swiftapps/unwired-mail/unwired-mailTests/ProductSyncEncryptionTests.swiftapps/unwired-mail/unwired-mailTests/ConvexClientTests.swiftapps/unwired-mail/unwired-mailTests/MessageCategorizationServiceTests.swiftapps/unwired-mail/unwired-mail/Features/Account/AccountView.swiftapps/unwired-mail/unwired-mail/Features/Settings/SettingsScene.swiftapps/unwired-mail/unwired-mail/Services/ProductAccount/ProductAccountSession.swiftapps/unwired-mail/unwired-mail/Services/Convex/ConvexClient.swiftapps/unwired-mail/unwired-mailTests/ProductAccountSessionTests.swiftapps/unwired-mail/unwired-mailTests/ProductSyncKeyMaterialStoreTests.swiftapps/unwired-mail/unwired-mail/Services/ProductSync/ProductSyncEncryption.swiftapps/unwired-mail/unwired-mail/Services/ProductAccount/ProductAccountService.swift
apps/unwired-mail/**/*.swift
📄 CodeRabbit inference engine (AGENTS.md)
SwiftLint runs in strict mode so warnings fail validation; use mise-managed SwiftLint and Xcode's
xcrun swift-formatwhere applicable.
Files:
apps/unwired-mail/unwired-mail/Services/ProductSync/MailboxConnectionSyncService.swiftapps/unwired-mail/unwired-mailTests/PinSyncServiceTests.swiftapps/unwired-mail/unwired-mail/Services/ProductSync/NotificationRuleSyncService.swiftapps/unwired-mail/unwired-mail/Services/ProductSync/MailboxConnectionSyncModels.swiftapps/unwired-mail/unwired-mail/Services/ProductSync/PinSyncService.swiftapps/unwired-mail/unwired-mailTests/GmailMessageMetadataServiceTests.swiftapps/unwired-mail/unwired-mail/Services/ProductSync/CustomCategorySyncService.swiftapps/unwired-mail/unwired-mailTests/CustomCategorySyncServiceTests.swiftapps/unwired-mail/unwired-mail/Services/Gmail/MessageCategorizationService.swiftapps/unwired-mail/unwired-mailTests/NotificationRuleSyncServiceTests.swiftapps/unwired-mail/unwired-mailTests/MailboxConnectionSyncServiceTests.swiftapps/unwired-mail/unwired-mailTests/ProductSyncEncryptionTests.swiftapps/unwired-mail/unwired-mailTests/ConvexClientTests.swiftapps/unwired-mail/unwired-mailTests/MessageCategorizationServiceTests.swiftapps/unwired-mail/unwired-mail/Features/Account/AccountView.swiftapps/unwired-mail/unwired-mail/Features/Settings/SettingsScene.swiftapps/unwired-mail/unwired-mail/Services/ProductAccount/ProductAccountSession.swiftapps/unwired-mail/unwired-mail/Services/Convex/ConvexClient.swiftapps/unwired-mail/unwired-mailTests/ProductAccountSessionTests.swiftapps/unwired-mail/unwired-mailTests/ProductSyncKeyMaterialStoreTests.swiftapps/unwired-mail/unwired-mail/Services/ProductSync/ProductSyncEncryption.swiftapps/unwired-mail/unwired-mail/Services/ProductAccount/ProductAccountService.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/MailboxConnectionSyncService.swiftapps/unwired-mail/unwired-mailTests/PinSyncServiceTests.swiftapps/unwired-mail/unwired-mail/Services/ProductSync/NotificationRuleSyncService.swiftapps/unwired-mail/unwired-mail/Services/ProductSync/MailboxConnectionSyncModels.swiftapps/unwired-mail/unwired-mail/Services/ProductSync/PinSyncService.swiftapps/unwired-mail/unwired-mailTests/GmailMessageMetadataServiceTests.swiftapps/unwired-mail/unwired-mail/Services/ProductSync/CustomCategorySyncService.swiftapps/unwired-mail/unwired-mailTests/CustomCategorySyncServiceTests.swiftapps/unwired-mail/unwired-mail/Services/Gmail/MessageCategorizationService.swiftapps/unwired-mail/unwired-mailTests/NotificationRuleSyncServiceTests.swiftapps/unwired-mail/unwired-mailTests/MailboxConnectionSyncServiceTests.swiftapps/unwired-mail/unwired-mailTests/ProductSyncEncryptionTests.swiftapps/unwired-mail/unwired-mailTests/ConvexClientTests.swiftapps/unwired-mail/unwired-mailTests/MessageCategorizationServiceTests.swiftapps/unwired-mail/unwired-mail/Features/Account/AccountView.swiftapps/unwired-mail/unwired-mail/Features/Settings/SettingsScene.swiftapps/unwired-mail/unwired-mail/Services/ProductAccount/ProductAccountSession.swiftapps/unwired-mail/unwired-mail/Services/Convex/ConvexClient.swiftapps/unwired-mail/unwired-mailTests/ProductAccountSessionTests.swiftapps/unwired-mail/unwired-mailTests/ProductSyncKeyMaterialStoreTests.swiftapps/unwired-mail/unwired-mail/Services/ProductSync/ProductSyncEncryption.swiftapps/unwired-mail/unwired-mail/Services/ProductAccount/ProductAccountService.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/PinSyncServiceTests.swiftapps/unwired-mail/unwired-mailTests/GmailMessageMetadataServiceTests.swiftapps/unwired-mail/unwired-mailTests/CustomCategorySyncServiceTests.swiftapps/unwired-mail/unwired-mailTests/NotificationRuleSyncServiceTests.swiftapps/unwired-mail/unwired-mailTests/MailboxConnectionSyncServiceTests.swiftapps/unwired-mail/unwired-mailTests/ProductSyncEncryptionTests.swiftapps/unwired-mail/unwired-mailTests/ConvexClientTests.swiftapps/unwired-mail/unwired-mailTests/MessageCategorizationServiceTests.swiftapps/unwired-mail/unwired-mailTests/ProductAccountSessionTests.swiftapps/unwired-mail/unwired-mailTests/ProductSyncKeyMaterialStoreTests.swift
**/*.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/0020-revoke-devices-with-sync-key-rotation.mdREADME.md
.changeset/*.md
📄 CodeRabbit inference engine (AGENTS.md)
Create a Changeset for runtime behavior changes or exported type/API changes using the package name and an appropriate patch, minor, or major bump; tests-only, internal refactors, docs-only changes, and JSDoc-only maintenance may omit one by maintainer decision.
Files:
.changeset/rotate-trusted-device-keys.md
packages/convex/convex/**/*.ts
📄 CodeRabbit inference engine (packages/convex/CLAUDE.md)
For work on Convex code, always read
convex/_generated/ai/guidelines.mdfirst; its rules override general Convex knowledge when using Convex APIs and patterns.For Convex code, always read
convex/_generated/ai/guidelines.mdfirst; its rules override general Convex knowledge when using Convex APIs and patterns.
Files:
packages/convex/convex/schema.tspackages/convex/convex/productAccountAuth.tspackages/convex/convex/productSync.tspackages/convex/convex/productAccount.ts
⚙️ CodeRabbit configuration file
packages/convex/convex/**/*.ts: Treat authentication, authorization, input validation, and privacy
boundaries as mandatory. Flag public functions that do not validate
arguments and returns, data access that is not scoped to the authenticated
Product Account or trusted device, plaintext sensitive data, secrets in
logs, unbounded reads, and queries that bypass appropriate indexes.
Changes to runtime behavior should include focused Convex tests covering
success, unauthorized access, malformed input, and relevant retries.
Files:
packages/convex/convex/schema.tspackages/convex/convex/productAccountAuth.tspackages/convex/convex/productSync.tspackages/convex/convex/productAccount.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: TypeScript linting must use Oxlint with@rajzik/oxlint-config.
TypeScript formatting must use Oxfmt with@rajzik/oxfmt-config.
TypeScript configuration must extend@rajzik/tsconfig.
Files:
packages/convex/convex/schema.tspackages/convex/convex/productAccountAuth.tspackages/convex/test/productAccount.test.tspackages/convex/test/productSync.test.tspackages/convex/convex/productSync.tspackages/convex/convex/productAccount.ts
packages/*/test/**/*.ts
⚙️ CodeRabbit configuration file
packages/*/test/**/*.ts: Verify externally observable behavior, authorization and privacy
invariants, malformed inputs, and deterministic isolation. Flag tests that
duplicate the implementation or omit meaningful negative cases.
Files:
packages/convex/test/productAccount.test.tspackages/convex/test/productSync.test.ts
🪛 GitHub Check: Fallow
packages/convex/convex/productAccount.ts
[failure] 777-777: High CRAP score (critical)
Function 'handler' has a CRAP score of 106.4 (threshold: 30).
• Severity: critical
• Cyclomatic: 20
• Cognitive: 23
• CRAP: 106.4 (threshold: 30)
• Lines: 151
CRAP combines complexity with coverage: high CRAP means changes here carry high risk.
Consider adding tests, simplifying the function, or both.
🪛 markdownlint-cli2 (0.23.1)
.changeset/rotate-trusted-device-keys.md
[warning] 5-5: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
🔇 Additional comments (49)
apps/unwired-mail/unwired-mailTests/ConvexClientTests.swift (1)
644-644: LGTM!Also applies to: 668-669, 701-706, 719-720, 743-743, 756-757, 786-787, 832-875
apps/unwired-mail/unwired-mailTests/CustomCategorySyncServiceTests.swift (1)
220-221: LGTM!Also applies to: 232-242
apps/unwired-mail/unwired-mailTests/MailboxConnectionSyncServiceTests.swift (1)
1301-1311: LGTM!Also applies to: 1320-1321, 1425-1442
apps/unwired-mail/unwired-mailTests/MessageCategorizationServiceTests.swift (1)
2370-2388: LGTM!apps/unwired-mail/unwired-mailTests/NotificationRuleSyncServiceTests.swift (1)
834-844: LGTM!Also applies to: 854-855
apps/unwired-mail/unwired-mailTests/PinSyncServiceTests.swift (1)
658-659: LGTM!Also applies to: 671-672, 702-703
apps/unwired-mail/unwired-mailTests/ProductSyncEncryptionTests.swift (1)
37-69: LGTM!Also applies to: 71-99
apps/unwired-mail/unwired-mailTests/ProductSyncKeyMaterialStoreTests.swift (1)
104-265: LGTM!Also applies to: 952-1009, 1055-1056
.changeset/rotate-trusted-device-keys.md (1)
1-7: LGTM!README.md (1)
125-125: LGTM!docs/adr/0020-revoke-devices-with-sync-key-rotation.md (1)
3-5: LGTM!packages/convex/convex/productAccountAuth.ts (1)
1-18: LGTM!Also applies to: 64-87
packages/convex/convex/productSync.ts (1)
191-197: LGTM!Also applies to: 247-253, 299-305, 323-329
packages/convex/test/productAccount.test.ts (1)
501-513: 📐 Maintainability & Code Quality | 💤 Low valueThe asserted arguments do not affect the result.
This call passes
encryptedTransition.keyVersion: 2andrecoveryWrappedAccountKey.keyVersion: 3, but the handler takes the pending-rotation branch and ignores both arguments. The committed recovery material is the epoch-2 payload stored by the first revocation, which the assertion on line 524 confirms. A reader can conclude that the epoch-3 material was validated and discarded.Add a comment that states the pending branch ignores the supplied material, or pass the material that matches the active rotation. Also confirm the intent: during a pending rotation the backend accepts any well-formed transition and recovery payload without comparing them to the stored pending material.
Source: Path instructions
packages/convex/test/productSync.test.ts (1)
89-89: LGTM!Also applies to: 118-118, 292-300, 330-330, 361-369, 382-384, 402-406, 426-426, 443-449, 460-460, 551-572
apps/unwired-mail/unwired-mail/Services/ProductAccount/ProductAccountService.swift (6)
29-45: LGTM!Also applies to: 53-67
85-121: LGTM!
227-264: LGTM!
300-331: LGTM!
519-523: LGTM!Also applies to: 527-559, 622-623, 659-660, 733-734, 755-768, 807-807
364-383: 🗄️ Data Integrity & IntegrationDo not flag this path.
revokeTrustedDevicedoes not writeargs.recoveryWrappedAccountKeywhen it returns an existing revocation. It stores new recovery material as pending data and commits it only when the rotation completes. The unchanged-epoch return does not corrupt Recovery Key state.> Likely an incorrect or invalid review comment.apps/unwired-mail/unwired-mail/Services/ProductSync/ProductSyncEncryption.swift (4)
79-136: LGTM!
146-174: LGTM!
325-371: LGTM!Also applies to: 380-401
403-441: LGTM!Also applies to: 443-479, 481-510
apps/unwired-mail/unwired-mail/Services/Convex/ConvexClient.swift (3)
8-8: LGTM!Also applies to: 19-20, 792-813
122-168: LGTM!Also applies to: 643-659
404-463: LGTM!Also applies to: 750-768
apps/unwired-mail/unwired-mail/Services/ProductSync/CustomCategorySyncService.swift (1)
19-34: LGTM!Also applies to: 190-198
apps/unwired-mail/unwired-mail/Services/ProductSync/MailboxConnectionSyncModels.swift (2)
120-131: LGTM!
106-113: 🗄️ Data Integrity & IntegrationKeep the account-wide payload listing.
listEncryptedPayloadsfilters byproductAccountId;trustedDeviceIdonly authenticates the device.> Likely an incorrect or invalid review comment.apps/unwired-mail/unwired-mail/Services/ProductSync/MailboxConnectionSyncService.swift (1)
426-434: LGTM!Also applies to: 436-452
apps/unwired-mail/unwired-mail/Services/ProductSync/PinSyncService.swift (1)
101-108: LGTM!Also applies to: 136-141
apps/unwired-mail/unwired-mail/Services/Gmail/MessageCategorizationService.swift (1)
601-605: LGTM!Also applies to: 637-641, 756-760, 870-874
apps/unwired-mail/unwired-mail/Services/ProductSync/NotificationRuleSyncService.swift (1)
298-299: LGTM!apps/unwired-mail/unwired-mail/Features/Account/AccountView.swift (2)
778-778: LGTM!Also applies to: 788-789
1531-1541: LGTM!apps/unwired-mail/unwired-mail/Features/Settings/SettingsScene.swift (3)
1115-1123: LGTM!Also applies to: 1139-1139
1171-1171: LGTM!Also applies to: 1183-1189, 1391-1414
1272-1301: LGTM!apps/unwired-mail/unwired-mail/Services/ProductAccount/ProductAccountSession.swift (5)
43-43: LGTM!Also applies to: 372-372, 663-663
150-159: LGTM!
401-402: LGTM!Also applies to: 546-546, 586-587
606-610: LGTM!Also applies to: 682-686
706-720: LGTM!Also applies to: 754-755, 766-768
apps/unwired-mail/unwired-mailTests/GmailMessageMetadataServiceTests.swift (1)
2114-2125: LGTM!apps/unwired-mail/unwired-mailTests/ProductAccountSessionTests.swift (3)
1866-1899: LGTM!Also applies to: 1944-1990
2546-2546: LGTM!Also applies to: 2562-2628, 2645-2669
2782-2782: LGTM!Also applies to: 2800-2802, 2817-2817, 2827-2828
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 425aba3b6b
ℹ️ 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.
Actionable comments posted: 3
♻️ Duplicate comments (4)
apps/unwired-mail/unwired-mail/Features/Settings/SettingsScene.swift (2)
1453-1458: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd a device-specific accessibility label to the "Revoke" button.
VoiceOver currently announces only "Revoke" for each device. Add a
revokeDevice(_:)accessibility helper besiderenameDevice(_:)and apply it to this button.🤖 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/Settings/SettingsScene.swift` around lines 1453 - 1458, Add a device-specific accessibility helper named revokeDevice(_:) alongside renameDevice(_:), then apply it to the “Revoke” Button in the trusted-device row so VoiceOver announces the action together with the target device.
993-1012: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winHandle
CancellationErrorinrevoke, same aspresentRecoveryKey.
recentIdentityToken()throwsCancellationErrorwhen the session is no longer current.revokemaps every error toerrorMessage, so a cancelled revocation shows an opaque alert.presentRecoveryKey(Line 1079) already treatsCancellationErroras a silent outcome. Apply the same handling here.🐛 Proposed fix for cancelled revocation
devices.removeAll { $0.id == device.id } pendingKeyRotationDeviceCount = response.pendingDeviceCount errorMessage = nil + } catch is CancellationError { } catch { errorMessage = error.localizedDescription }🤖 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/Settings/SettingsScene.swift` around lines 993 - 1012, Update revoke to catch CancellationError separately from other failures, matching presentRecoveryKey’s silent cancellation handling; leave errorMessage unset for cancellation while retaining the existing localized error assignment for non-cancellation errors.apps/unwired-mail/unwired-mailTests/ProductAccountSessionTests.swift (1)
1901-1942: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd a test for a transient failure during foreground revalidation.
revalidateTrustedDeviceAfterForegroundingdiscards non-revocation errors so a connectivity failure does not destroy a valid session. No test covers that branch. Add a case whereconnectthrows a transient error after a successful bootstrap, then assert the session stays.signedInwith the original snapshot, the session store keeps the snapshot, and the key material survives.As per path instructions: "Verify externally observable behavior, privacy invariants, error paths, cancellation or restart behavior where relevant."
Also applies to: 1944-1991, 1992-2037
🤖 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-mailTests/ProductAccountSessionTests.swift` around lines 1901 - 1942, Add a ProductAccountSession test alongside testForegroundRevalidationPurgesLocalDataAfterRemoteRevocation that bootstraps successfully, makes the revalidation connect operation throw a transient error, and verifies revalidateTrustedDeviceAfterForegrounding preserves the .signedIn state, original snapshot in the session store, and product sync key material. Use the existing test doubles or introduce the minimal failure configuration needed to target the non-revocation error path without changing revocation behavior.Source: Path instructions
apps/unwired-mail/unwired-mail/Services/ProductAccount/ProductAccountSession.swift (1)
101-151: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftSame-account device re-registration is unhandled in production code and untested.
revalidateTrustedDeviceAfterForegroundingtreats a same-productAccountId, different-trustedDeviceIdresponse asdifferentAppleAccountand silently discards it via the general catch, and no test exercises this path.
apps/unwired-mail/unwired-mail/Services/ProductAccount/ProductAccountSession.swift#L101-L151: handle the same-account, different-trustedDeviceIdresponse as re-registration — adopt the newtrustedDeviceIdand refresh the session — instead of throwingdifferentAppleAccountand swallowing it.apps/unwired-mail/unwired-mailTests/ProductAccountSessionTests.swift#L1992-L2037: add a test alongsidetestForegroundRevalidationRejectsAnotherAppleAccountBeforeConnectingthat keeps the sameappleUserIdentifierandproductAccountIdbut returns a differenttrustedDeviceIdfromconnect, asserting the session adopts the new device id instead of getting stuck or silently failing.🤖 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/ProductAccount/ProductAccountSession.swift` around lines 101 - 151, Update revalidateTrustedDeviceAfterForegrounding in apps/unwired-mail/unwired-mail/Services/ProductAccount/ProductAccountSession.swift (lines 101-151) to treat a response with the same productAccountId but a new trustedDeviceId as valid device re-registration: adopt the returned device ID and refresh/save the session instead of throwing differentAppleAccount. Add a sibling test in apps/unwired-mail/unwired-mailTests/ProductAccountSessionTests.swift (lines 1992-2037), alongside testForegroundRevalidationRejectsAnotherAppleAccountBeforeConnecting, using the same Apple account and product account with a different returned trustedDeviceId and asserting the session adopts it.
🤖 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 1332-1336: Update the foregrounding Task flow around
revalidateTrustedDeviceAfterForegrounding and synchronizeMailboxesFully so token
refreshes do not cause the continuation to exit through
session.isCurrent(snapshot). Replace the full snapshot comparison with a stable
account/device identity check, or refresh the snapshot after revalidation before
validating it, in both affected synchronization paths.
In `@apps/unwired-mail/unwired-mail/Features/Settings/SettingsScene.swift`:
- Around line 959-961: Update revoke() to guard on canRevokeTrustedDevices
before calling service.revokeDevice, returning a distinct error when the
recovery key status is not current. Add a focused companion test in
ProductSyncKeyMaterialStoreTests.swift that invokes revoke() directly with
canRevokeTrustedDevices false, verifies the distinct failure, and confirms the
service is not called.
In `@packages/convex/convex/productAccount.ts`:
- Around line 818-850: Update revokeDuringPendingKeyRotation in
packages/convex/convex/productAccount.ts (lines 818-850) to branch on
target.productSyncKeyEpoch: preserve the existing fast path when the target has
not adopted pendingKeyEpoch, but when it has, validate expectedRecoveryUpdatedAt
and start a follow-on epoch using encryptedTransition and
recoveryWrappedAccountKey, or return a distinct error if validation fails.
Update the corresponding assertions in
packages/convex/test/productAccount.test.ts (lines 588-600) to verify the
remaining devices’ protecting epoch and resulting recovery material, using a
newly produced recovery timestamp instead of the pre-revocation
recoveryMaterial.updatedAt.
---
Duplicate comments:
In `@apps/unwired-mail/unwired-mail/Features/Settings/SettingsScene.swift`:
- Around line 1453-1458: Add a device-specific accessibility helper named
revokeDevice(_:) alongside renameDevice(_:), then apply it to the “Revoke”
Button in the trusted-device row so VoiceOver announces the action together with
the target device.
- Around line 993-1012: Update revoke to catch CancellationError separately from
other failures, matching presentRecoveryKey’s silent cancellation handling;
leave errorMessage unset for cancellation while retaining the existing localized
error assignment for non-cancellation errors.
In
`@apps/unwired-mail/unwired-mail/Services/ProductAccount/ProductAccountSession.swift`:
- Around line 101-151: Update revalidateTrustedDeviceAfterForegrounding in
apps/unwired-mail/unwired-mail/Services/ProductAccount/ProductAccountSession.swift
(lines 101-151) to treat a response with the same productAccountId but a new
trustedDeviceId as valid device re-registration: adopt the returned device ID
and refresh/save the session instead of throwing differentAppleAccount. Add a
sibling test in
apps/unwired-mail/unwired-mailTests/ProductAccountSessionTests.swift (lines
1992-2037), alongside
testForegroundRevalidationRejectsAnotherAppleAccountBeforeConnecting, using the
same Apple account and product account with a different returned trustedDeviceId
and asserting the session adopts it.
In `@apps/unwired-mail/unwired-mailTests/ProductAccountSessionTests.swift`:
- Around line 1901-1942: Add a ProductAccountSession test alongside
testForegroundRevalidationPurgesLocalDataAfterRemoteRevocation that bootstraps
successfully, makes the revalidation connect operation throw a transient error,
and verifies revalidateTrustedDeviceAfterForegrounding preserves the .signedIn
state, original snapshot in the session store, and product sync key material.
Use the existing test doubles or introduce the minimal failure configuration
needed to target the non-revocation error path without changing revocation
behavior.
🪄 Autofix (Beta)
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: 6a031e16-b7dd-4af7-a961-b70792f59dc9
📒 Files selected for processing (8)
apps/unwired-mail/unwired-mail/Features/Account/AccountView.swiftapps/unwired-mail/unwired-mail/Features/Settings/SettingsScene.swiftapps/unwired-mail/unwired-mail/Services/ProductAccount/ProductAccountSession.swiftapps/unwired-mail/unwired-mailTests/ProductAccountSessionTests.swiftapps/unwired-mail/unwired-mailTests/ProductSyncKeyMaterialStoreTests.swiftpackages/convex/convex/productAccount.tspackages/convex/convex/productSync.tspackages/convex/test/productAccount.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Apple · Unwired Mail
🧰 Additional context used
📓 Path-based instructions (6)
apps/unwired-mail/**/*.{swift,mm,m,h}
📄 CodeRabbit inference engine (AGENTS.md)
The Apple app must provide formatter, linter, and test commands; use
zsh scripts/check-apple-lint.zshfor formatting and linting and the specifiedxcodebuild testcommand for tests.
Files:
apps/unwired-mail/unwired-mailTests/ProductSyncKeyMaterialStoreTests.swiftapps/unwired-mail/unwired-mail/Services/ProductAccount/ProductAccountSession.swiftapps/unwired-mail/unwired-mail/Features/Account/AccountView.swiftapps/unwired-mail/unwired-mail/Features/Settings/SettingsScene.swiftapps/unwired-mail/unwired-mailTests/ProductAccountSessionTests.swift
apps/unwired-mail/**/*.swift
📄 CodeRabbit inference engine (AGENTS.md)
SwiftLint runs in strict mode so warnings fail validation; use mise-managed SwiftLint and Xcode's
xcrun swift-formatwhere applicable.
Files:
apps/unwired-mail/unwired-mailTests/ProductSyncKeyMaterialStoreTests.swiftapps/unwired-mail/unwired-mail/Services/ProductAccount/ProductAccountSession.swiftapps/unwired-mail/unwired-mail/Features/Account/AccountView.swiftapps/unwired-mail/unwired-mail/Features/Settings/SettingsScene.swiftapps/unwired-mail/unwired-mailTests/ProductAccountSessionTests.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/ProductSyncKeyMaterialStoreTests.swiftapps/unwired-mail/unwired-mail/Services/ProductAccount/ProductAccountSession.swiftapps/unwired-mail/unwired-mail/Features/Account/AccountView.swiftapps/unwired-mail/unwired-mail/Features/Settings/SettingsScene.swiftapps/unwired-mail/unwired-mailTests/ProductAccountSessionTests.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/ProductSyncKeyMaterialStoreTests.swiftapps/unwired-mail/unwired-mailTests/ProductAccountSessionTests.swift
packages/convex/convex/**/*.ts
📄 CodeRabbit inference engine (packages/convex/CLAUDE.md)
For work on Convex code, always read
convex/_generated/ai/guidelines.mdfirst; its rules override general Convex knowledge when using Convex APIs and patterns.For Convex code, always read
convex/_generated/ai/guidelines.mdfirst; its rules override general Convex knowledge when using Convex APIs and patterns.
Files:
packages/convex/convex/productAccount.tspackages/convex/convex/productSync.ts
⚙️ CodeRabbit configuration file
packages/convex/convex/**/*.ts: Treat authentication, authorization, input validation, and privacy
boundaries as mandatory. Flag public functions that do not validate
arguments and returns, data access that is not scoped to the authenticated
Product Account or trusted device, plaintext sensitive data, secrets in
logs, unbounded reads, and queries that bypass appropriate indexes.
Changes to runtime behavior should include focused Convex tests covering
success, unauthorized access, malformed input, and relevant retries.
Files:
packages/convex/convex/productAccount.tspackages/convex/convex/productSync.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: TypeScript linting must use Oxlint with@rajzik/oxlint-config.
TypeScript formatting must use Oxfmt with@rajzik/oxfmt-config.
TypeScript configuration must extend@rajzik/tsconfig.
Files:
packages/convex/convex/productAccount.tspackages/convex/test/productAccount.test.tspackages/convex/convex/productSync.ts
packages/*/test/**/*.ts
⚙️ CodeRabbit configuration file
packages/*/test/**/*.ts: Verify externally observable behavior, authorization and privacy
invariants, malformed inputs, and deterministic isolation. Flag tests that
duplicate the implementation or omit meaningful negative cases.
Files:
packages/convex/test/productAccount.test.ts
🔇 Additional comments (12)
packages/convex/convex/productAccount.ts (6)
905-911:productSyncPendingRevokedDeviceIdstill has no reader, andrevokeDuringPendingKeyRotationdoes not set it, so it records only the first revoked device of a rotation. Remove the field, or populate it consistently in both revocation paths.
773-788: LGTM!
790-809: LGTM!
858-904: LGTM!Also applies to: 912-917
957-975: LGTM!
1-2: LGTM!packages/convex/convex/productSync.ts (3)
88-116: LGTM!
118-138: LGTM!
238-241: LGTM!packages/convex/test/productAccount.test.ts (1)
527-587: LGTM!apps/unwired-mail/unwired-mail/Features/Account/AccountView.swift (1)
1530-1540: 🩺 Stability & AvailabilityVerify
pollWhileActiverejects synchronization after a stale revalidation result.
revalidateTrustedDevicewires directly tosession.revalidateTrustedDeviceAfterForegrounding(), anddidSynchronizecallsreloadObservedMailboxes()unconditionally afterward. This file does not gatedidSynchronizeonsession.isCurrent(snapshot).If
MailboxFreshnessViewModel.pollWhileActivedoes not itself verify session validity before invokingdidSynchronize, this closure could runreloadObservedMailboxes()after the trusted device was revoked and the session moved to.signedOut, or it could hit the same stale-snapshot comparison described for lines 1332-1336 and 1541-1550 if it performs an equivalent check internally.Confirm that
pollWhileActivechecks session/device validity after callingrevalidateTrustedDeviceand before callingdidSynchronize, and that this check tolerates a legitimately refreshed identity token.apps/unwired-mail/unwired-mailTests/ProductSyncKeyMaterialStoreTests.swift (1)
104-146: LGTM!Also applies to: 148-210, 212-264, 334-360, 1081-1092
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: af0ac87907
ℹ️ 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 (10)
apps/unwired-mail/unwired-mail/Features/Settings/SettingsScene.swift (1)
1168-1173: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winName the provider authorization instead of only Gmail and Microsoft sessions.
The footer tells the user to revoke "Gmail or Microsoft sessions". The app also supports Exchange Web Services and Generic Mail connections, and
CONTEXT.mddefines the concept as Provider Mailbox Authorization revoked through the Mail Provider. A user with an Exchange or Generic Mail connection receives no guidance for the mailbox credentials that stay on the revoked device.State that Provider Mailbox Authorization must be revoked separately with each Mail Provider, and keep the Gmail and Microsoft references as examples.
As per coding guidelines: "Every change must consider documentation and update relevant documentation when behavior, setup, commands, architecture, environment variables, public interfaces, or agent workflow expectations change."
🤖 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/Settings/SettingsScene.swift` around lines 1168 - 1173, Update the trustedDevicesFooter text to state that Provider Mailbox Authorization must also be revoked separately with each Mail Provider, while retaining Gmail and Microsoft sessions as examples. Ensure the wording covers Exchange Web Services and Generic Mail connections without changing the surrounding revocation guidance.Source: Coding guidelines
apps/unwired-mail/unwired-mail/Services/Convex/ConvexClient.swift (1)
536-545: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePreserve the server message when an application error code is present.
The HTTP-action path decodes
ConvexFunctionErrorDataand then discards the message.ConvexClientError.convexApplicationFailurethen reports the generic text "The backend rejected the request." The function-call path keepsenvelope.errorMessageinfailure(for:). Pass the decoded message here so both transports report the same detail.♻️ Proposed change
), let code = errorData.code { throw ConvexClientError.convexApplicationFailure( status: "error", code: code, - message: nil + message: errorData.message ) }Confirm the property name on
ConvexFunctionErrorDatabefore applying.🤖 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/Convex/ConvexClient.swift` around lines 536 - 545, Update the HTTP-action error handling around ConvexFunctionErrorData decoding to pass its decoded message into ConvexClientError.convexApplicationFailure instead of nil. Confirm and use the actual message property name on ConvexFunctionErrorData, preserving the existing status and code values so HTTP-action errors match failure(for:).apps/unwired-mail/unwired-mail/Services/ProductAccount/ProductAccountService.swift (1)
515-529: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReport a distinct outcome when revocation commits but the follow-up read fails.
transport.revokeTrustedDevicehas already committed the revocation and the new key epoch at this point. If the followingproductSyncKeyRotationread fails or returns a different epoch, the code throwsAccountAndDevicesServiceError.recoveryMaterialChanged.SettingsScene.revokeshows that error text, which states the Recovery Key changed on another Trusted Device and asks the user to retry. The revocation is not reversible, so the text misdescribes the state and invites a pointless retry.Add a dedicated error case for a committed revocation with an incomplete local rotation, and let the next reconciliation adopt the epoch.
🤖 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/ProductAccount/ProductAccountService.swift` around lines 515 - 529, Add a dedicated AccountAndDevicesServiceError case for a committed revocation whose follow-up key-rotation read fails or has a mismatched epoch, and throw it from the authoritativeRotation guard instead of recoveryMaterialChanged. Update the surrounding reconciliation flow to preserve the committed revocation and let the next reconciliation adopt the authoritative epoch, while retaining recoveryMaterialChanged for genuine concurrent changes.apps/unwired-mail/unwired-mail/Features/Account/AccountView.swift (1)
1124-1127: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
GenericMailSetupViewModel.isSessionCurrentvalidates an init-captured snapshot. Both call sites build the closure ininitand store the view model in@State, so the capturedsnapshotis never replaced.updateSession(_:)refreshes onlysyncSession. After foreground revalidation refreshes the identity token or adopts a newtrustedDeviceId, the predicate returnsfalseand Generic Mail connect and clear operations stop working until the view is recreated.
apps/unwired-mail/unwired-mail/Features/Account/AccountView.swift#L1124-L1127: evaluate the model's current session instead of the capturedsnapshot, matchingisSyncSessionCurrent.apps/unwired-mail/unwired-mail/Features/Settings/SettingsScene.swift#L1765-L1768: replacesession.isCurrent(snapshot)with a current-session identity check, becauseisCurrentalso comparesidentityToken.Add a focused test that refreshes the session and then connects a Generic Mail definition.
🤖 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 1124 - 1127, Update GenericMailSetupViewModel’s isSessionCurrent closures in apps/unwired-mail/unwired-mail/Features/Account/AccountView.swift:1124-1127 and apps/unwired-mail/unwired-mail/Features/Settings/SettingsScene.swift:1765-1768 to evaluate the model’s current session identity rather than the init-captured snapshot; retain isSyncSessionCurrent’s current-session behavior and avoid session.isCurrent, which also compares identityToken. Add a focused test that refreshes the session and then connects a Generic Mail definition.apps/unwired-mail/unwired-mail/Services/ProductAccount/ProductAccountSession.swift (2)
313-316: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftAllow cached reads after transient revalidation failures.
The generic catch returns
falsefor errors such asConvexClientErrorfromproductAccountService.connect.AccountViewthen cancels cached body and attachment reads, even though the signed-in session remains valid. Return a result that distinguishes transient failures from invalid session state. Allow cached reads during transient failures while continuing to block synchronization and mutations. Add offline-foreground tests for cached body and attachment reads.🤖 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/ProductAccount/ProductAccountSession.swift` around lines 313 - 316, The catch in ProductAccountSession’s revalidation flow must distinguish transient connectivity or ConvexClientError failures from invalid session state instead of returning false for all errors. Return the existing session-result state that permits cached reads while still preventing synchronization and mutations, and preserve false for genuinely invalid sessions. Add offline-foreground coverage for cached body and attachment reads through AccountView.Source: Path instructions
1349-1361: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winClear the notification state and the session even when mailbox cleanup fails.
clearRevokedSessionreturns early whenmailboxConnectionService.clearLocalConnectionthrows. It then skipsnotificationClearer.clear, the unregistration retry marker, andresumePendingSignOut, which performssessionStore.clear, Product Sync cache clearing, and key-material clearing. The revoked device keeps the session snapshot, cached Product Sync material, and local notifications until the next launch resumes the pending sign-out marker.Continue the remaining cleanup steps and return the first error, so the purge is as complete as possible in the same run.
As per path instructions: "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."
🤖 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/ProductAccount/ProductAccountSession.swift` around lines 1349 - 1361, Update clearRevokedSession to retain the mailbox cleanup error while continuing notificationClearer.clear, persistTrustedDeviceUnregistrationRetry, and resumePendingSignOut so session, Product Sync cache, and key material are cleared in the same run. Return the first encountered error only after all cleanup steps have been attempted, and do not log or persist credentials, tokens, mail content, or decrypted Product Sync data outside approved device-local storage.Source: Path instructions
apps/unwired-mail/unwired-mailTests/ConvexClientTests.swift (1)
813-824: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winAssert the device-scoped query path for the single-payload read.
This test now verifies the
trustedDeviceIdargument, but it does not verifypath. The targeted variant at Line 858 assertsproductSync:getEncryptedPayloadsForTrustedDevice. IfgetEncryptedProductSyncPayloadregressed to an account-scoped Convex function while still forwardingtrustedDeviceId, this test would stay green. Add the path assertion.As per path instructions: "Verify externally observable behavior, privacy invariants, error paths, cancellation or restart behavior where relevant, and deterministic isolation."
🛡️ Proposed additional assertion
let args = try XCTUnwrap(requestJSON["args"] as? [String: Any]) + XCTAssertEqual( + requestJSON["path"] as? String, + "productSync:getEncryptedPayloadForTrustedDevice" + ) XCTAssertEqual(args["trustedDeviceId"] as? String, "trusted-device-001")Use the exact function name that
ConvexClient.getEncryptedProductSyncPayloadcalls.🤖 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-mailTests/ConvexClientTests.swift` around lines 813 - 824, Add an assertion in the request validation closure for the single-payload read test to verify requestJSON["path"] equals the exact device-scoped Convex function invoked by ConvexClient.getEncryptedProductSyncPayload. Keep the existing trustedDeviceId and request checks unchanged.Source: Path instructions
apps/unwired-mail/unwired-mailTests/MailboxConnectionAdapterTests.swift (1)
753-769: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExercise the refreshed session instead of the stored property.
This test writes
sessionSnapshotand reads it back. It restates the property implementation and passes for any storage, so it does not pin the refresh contract. Assert that a later view-model operation uses the refreshed identity token, for example by asserting the session thatRecordingAdapterConnectionServicereceives duringload()after the refresh.As per path instructions: "Flag fragile sleeps, ordering dependencies, and tests that duplicate the implementation instead of exercising it."
🤖 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-mailTests/MailboxConnectionAdapterTests.swift` around lines 753 - 769, The test testViewModelUpdatesSessionAfterIdentityTokenRefresh currently only verifies property storage, duplicating the implementation. After assigning refreshedSession, invoke a later view-model operation such as load() and assert that the RecordingAdapterConnectionService receives a session containing the refreshed identity token, preserving the refresh behavior under test.Source: Path instructions
packages/convex/convex/schema.ts (1)
29-35: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftDo not persist provider credentials as plaintext.
revocationMaterial.valuestores authorization codes, access tokens, and refresh tokens as raw strings. A database read or export can expose usable Mail Provider authorization.Store only an encrypted secret reference or envelope-encrypted material. Delete the material immediately after the revocation attempt reaches a terminal state.
As per path instructions, flag “plaintext sensitive data” at Convex privacy boundaries.
🤖 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 `@packages/convex/convex/schema.ts` around lines 29 - 35, Update the revocationMaterial schema and its persistence flow to prohibit raw authorization codes, access tokens, and refresh tokens; store only an encrypted secret reference or envelope-encrypted material. Ensure revocationMaterial is deleted immediately after the revocation attempt reaches a terminal state, and flag plaintext sensitive data at the Convex privacy boundary.Source: Path instructions
packages/convex/convex/productAccountAuth.ts (1)
18-43: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winAdd a focused
deleting-dataaccess-fence test.Existing tests cover
revocationSucceededAtand tombstone fencing throughconnect. They do not assertPRODUCT_ACCOUNT_DELETEDwhilephaseisdeleting-dataor through a protected operation such aslistTrustedDevices.🤖 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 `@packages/convex/convex/productAccountAuth.ts` around lines 18 - 43, Add a focused test covering requireProductAccountNotDeleted when the deletion request phase is deleting-data, asserting the protected operation returns PRODUCT_ACCOUNT_DELETED. Exercise the fence through listTrustedDevices (or the nearest protected operation) and keep existing revocationSucceededAt and tombstone coverage unchanged.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 1124-1127: Update GenericMailSetupViewModel’s isSessionCurrent
closures in
apps/unwired-mail/unwired-mail/Features/Account/AccountView.swift:1124-1127 and
apps/unwired-mail/unwired-mail/Features/Settings/SettingsScene.swift:1765-1768
to evaluate the model’s current session identity rather than the init-captured
snapshot; retain isSyncSessionCurrent’s current-session behavior and avoid
session.isCurrent, which also compares identityToken. Add a focused test that
refreshes the session and then connects a Generic Mail definition.
In `@apps/unwired-mail/unwired-mail/Features/Settings/SettingsScene.swift`:
- Around line 1168-1173: Update the trustedDevicesFooter text to state that
Provider Mailbox Authorization must also be revoked separately with each Mail
Provider, while retaining Gmail and Microsoft sessions as examples. Ensure the
wording covers Exchange Web Services and Generic Mail connections without
changing the surrounding revocation guidance.
In `@apps/unwired-mail/unwired-mail/Services/Convex/ConvexClient.swift`:
- Around line 536-545: Update the HTTP-action error handling around
ConvexFunctionErrorData decoding to pass its decoded message into
ConvexClientError.convexApplicationFailure instead of nil. Confirm and use the
actual message property name on ConvexFunctionErrorData, preserving the existing
status and code values so HTTP-action errors match failure(for:).
In
`@apps/unwired-mail/unwired-mail/Services/ProductAccount/ProductAccountService.swift`:
- Around line 515-529: Add a dedicated AccountAndDevicesServiceError case for a
committed revocation whose follow-up key-rotation read fails or has a mismatched
epoch, and throw it from the authoritativeRotation guard instead of
recoveryMaterialChanged. Update the surrounding reconciliation flow to preserve
the committed revocation and let the next reconciliation adopt the authoritative
epoch, while retaining recoveryMaterialChanged for genuine concurrent changes.
In
`@apps/unwired-mail/unwired-mail/Services/ProductAccount/ProductAccountSession.swift`:
- Around line 313-316: The catch in ProductAccountSession’s revalidation flow
must distinguish transient connectivity or ConvexClientError failures from
invalid session state instead of returning false for all errors. Return the
existing session-result state that permits cached reads while still preventing
synchronization and mutations, and preserve false for genuinely invalid
sessions. Add offline-foreground coverage for cached body and attachment reads
through AccountView.
- Around line 1349-1361: Update clearRevokedSession to retain the mailbox
cleanup error while continuing notificationClearer.clear,
persistTrustedDeviceUnregistrationRetry, and resumePendingSignOut so session,
Product Sync cache, and key material are cleared in the same run. Return the
first encountered error only after all cleanup steps have been attempted, and do
not log or persist credentials, tokens, mail content, or decrypted Product Sync
data outside approved device-local storage.
In `@apps/unwired-mail/unwired-mailTests/ConvexClientTests.swift`:
- Around line 813-824: Add an assertion in the request validation closure for
the single-payload read test to verify requestJSON["path"] equals the exact
device-scoped Convex function invoked by
ConvexClient.getEncryptedProductSyncPayload. Keep the existing trustedDeviceId
and request checks unchanged.
In `@apps/unwired-mail/unwired-mailTests/MailboxConnectionAdapterTests.swift`:
- Around line 753-769: The test
testViewModelUpdatesSessionAfterIdentityTokenRefresh currently only verifies
property storage, duplicating the implementation. After assigning
refreshedSession, invoke a later view-model operation such as load() and assert
that the RecordingAdapterConnectionService receives a session containing the
refreshed identity token, preserving the refresh behavior under test.
In `@packages/convex/convex/productAccountAuth.ts`:
- Around line 18-43: Add a focused test covering requireProductAccountNotDeleted
when the deletion request phase is deleting-data, asserting the protected
operation returns PRODUCT_ACCOUNT_DELETED. Exercise the fence through
listTrustedDevices (or the nearest protected operation) and keep existing
revocationSucceededAt and tombstone coverage unchanged.
In `@packages/convex/convex/schema.ts`:
- Around line 29-35: Update the revocationMaterial schema and its persistence
flow to prohibit raw authorization codes, access tokens, and refresh tokens;
store only an encrypted secret reference or envelope-encrypted material. Ensure
revocationMaterial is deleted immediately after the revocation attempt reaches a
terminal state, and flag plaintext sensitive data at the Convex privacy
boundary.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9b8086cb-c099-4ce8-ba35-c66a8c110500
📒 Files selected for processing (15)
README.mdapps/unwired-mail/unwired-mail/Features/Account/AccountView.swiftapps/unwired-mail/unwired-mail/Features/Settings/SettingsScene.swiftapps/unwired-mail/unwired-mail/Services/Convex/ConvexClient.swiftapps/unwired-mail/unwired-mail/Services/ProductAccount/ProductAccountService.swiftapps/unwired-mail/unwired-mail/Services/ProductAccount/ProductAccountSession.swiftapps/unwired-mail/unwired-mail/Services/ProductAccount/ProductAccountSessionStore.swiftapps/unwired-mail/unwired-mailTests/ConvexClientTests.swiftapps/unwired-mail/unwired-mailTests/GenericMailSetupServiceTests.swiftapps/unwired-mail/unwired-mailTests/MailboxConnectionAdapterTests.swiftapps/unwired-mail/unwired-mailTests/ProductAccountSessionTests.swiftpackages/convex/convex/productAccount.tspackages/convex/convex/productAccountAuth.tspackages/convex/convex/schema.tspackages/convex/test/productAccount.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Apple · Unwired Mail
🧰 Additional context used
📓 Path-based instructions (11)
**/*
📄 CodeRabbit inference engine (AGENTS.md)
**/*: Inspect nearby implementation, tests, and pattern documentation before editing, and prefer existing abstractions and conventions.
Every change must consider documentation and update relevant documentation when behavior, setup, commands, architecture, environment variables, public interfaces, or agent workflow expectations change.
When addressing GitHub comments after fixing them and pushing PR changes, resolve the comments and reference the issue the PR solves.
Before finishing, confirm only intended files changed, run relevant validation, update required documentation, and summarize verified and unrun commands.
Files:
README.mdapps/unwired-mail/unwired-mail/Services/ProductAccount/ProductAccountSessionStore.swiftapps/unwired-mail/unwired-mailTests/GenericMailSetupServiceTests.swiftpackages/convex/convex/productAccountAuth.tsapps/unwired-mail/unwired-mail/Services/Convex/ConvexClient.swiftapps/unwired-mail/unwired-mailTests/MailboxConnectionAdapterTests.swiftapps/unwired-mail/unwired-mailTests/ConvexClientTests.swiftpackages/convex/test/productAccount.test.tsapps/unwired-mail/unwired-mail/Features/Settings/SettingsScene.swiftapps/unwired-mail/unwired-mail/Services/ProductAccount/ProductAccountService.swiftpackages/convex/convex/productAccount.tsapps/unwired-mail/unwired-mail/Services/ProductAccount/ProductAccountSession.swiftapps/unwired-mail/unwired-mailTests/ProductAccountSessionTests.swiftpackages/convex/convex/schema.tsapps/unwired-mail/unwired-mail/Features/Account/AccountView.swift
README.md
📄 CodeRabbit inference engine (AGENTS.md)
Use
README.mdfor developer setup, common commands, and high-level project navigation.
Files:
README.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:
README.md
apps/unwired-mail/**/*.{swift,plist}
📄 CodeRabbit inference engine (AGENTS.md)
apps/unwired-mail/**/*.{swift,plist}: Apple app code belongs underapps/unwired-mailand must provide formatter, linter, and test commands.
For Apple validation, runzsh scripts/check-apple-lint.zshfor formatting and linting, and run the specifiedxcodebuild testcommand for theunwired-mailiOS scheme.
Files:
apps/unwired-mail/unwired-mail/Services/ProductAccount/ProductAccountSessionStore.swiftapps/unwired-mail/unwired-mailTests/GenericMailSetupServiceTests.swiftapps/unwired-mail/unwired-mail/Services/Convex/ConvexClient.swiftapps/unwired-mail/unwired-mailTests/MailboxConnectionAdapterTests.swiftapps/unwired-mail/unwired-mailTests/ConvexClientTests.swiftapps/unwired-mail/unwired-mail/Features/Settings/SettingsScene.swiftapps/unwired-mail/unwired-mail/Services/ProductAccount/ProductAccountService.swiftapps/unwired-mail/unwired-mail/Services/ProductAccount/ProductAccountSession.swiftapps/unwired-mail/unwired-mailTests/ProductAccountSessionTests.swiftapps/unwired-mail/unwired-mail/Features/Account/AccountView.swift
**/*.{ts,tsx,swift}
📄 CodeRabbit inference engine (AGENTS.md)
Every code change must have appropriate lint, format, and test coverage before handoff unless a required tool or platform dependency is unavailable; start with the smallest meaningful checks and broaden for shared or cross-package changes.
Files:
apps/unwired-mail/unwired-mail/Services/ProductAccount/ProductAccountSessionStore.swiftapps/unwired-mail/unwired-mailTests/GenericMailSetupServiceTests.swiftpackages/convex/convex/productAccountAuth.tsapps/unwired-mail/unwired-mail/Services/Convex/ConvexClient.swiftapps/unwired-mail/unwired-mailTests/MailboxConnectionAdapterTests.swiftapps/unwired-mail/unwired-mailTests/ConvexClientTests.swiftpackages/convex/test/productAccount.test.tsapps/unwired-mail/unwired-mail/Features/Settings/SettingsScene.swiftapps/unwired-mail/unwired-mail/Services/ProductAccount/ProductAccountService.swiftpackages/convex/convex/productAccount.tsapps/unwired-mail/unwired-mail/Services/ProductAccount/ProductAccountSession.swiftapps/unwired-mail/unwired-mailTests/ProductAccountSessionTests.swiftpackages/convex/convex/schema.tsapps/unwired-mail/unwired-mail/Features/Account/AccountView.swift
apps/unwired-mail/**/*.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/ProductAccount/ProductAccountSessionStore.swiftapps/unwired-mail/unwired-mailTests/GenericMailSetupServiceTests.swiftapps/unwired-mail/unwired-mail/Services/Convex/ConvexClient.swiftapps/unwired-mail/unwired-mailTests/MailboxConnectionAdapterTests.swiftapps/unwired-mail/unwired-mailTests/ConvexClientTests.swiftapps/unwired-mail/unwired-mail/Features/Settings/SettingsScene.swiftapps/unwired-mail/unwired-mail/Services/ProductAccount/ProductAccountService.swiftapps/unwired-mail/unwired-mail/Services/ProductAccount/ProductAccountSession.swiftapps/unwired-mail/unwired-mailTests/ProductAccountSessionTests.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/GenericMailSetupServiceTests.swiftapps/unwired-mail/unwired-mailTests/MailboxConnectionAdapterTests.swiftapps/unwired-mail/unwired-mailTests/ConvexClientTests.swiftapps/unwired-mail/unwired-mailTests/ProductAccountSessionTests.swift
packages/convex/convex/**/*.ts
📄 CodeRabbit inference engine (packages/convex/CLAUDE.md)
For work on Convex code, always read
convex/_generated/ai/guidelines.mdfirst; its rules override general Convex knowledge when using Convex APIs and patterns.For Convex code, always read
convex/_generated/ai/guidelines.mdfirst; its rules override general Convex knowledge when using Convex APIs and patterns.
Files:
packages/convex/convex/productAccountAuth.tspackages/convex/convex/productAccount.tspackages/convex/convex/schema.ts
⚙️ CodeRabbit configuration file
packages/convex/convex/**/*.ts: Treat authentication, authorization, input validation, and privacy
boundaries as mandatory. Flag public functions that do not validate
arguments and returns, data access that is not scoped to the authenticated
Product Account or trusted device, plaintext sensitive data, secrets in
logs, unbounded reads, and queries that bypass appropriate indexes.
Changes to runtime behavior should include focused Convex tests covering
success, unauthorized access, malformed input, and relevant retries.
Files:
packages/convex/convex/productAccountAuth.tspackages/convex/convex/productAccount.tspackages/convex/convex/schema.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use pnpm and the repository TypeScript tooling: Oxlint with
@rajzik/oxlint-config, Oxfmt with@rajzik/oxfmt-config, and TypeScript configuration extending@rajzik/tsconfig.
Files:
packages/convex/convex/productAccountAuth.tspackages/convex/test/productAccount.test.tspackages/convex/convex/productAccount.tspackages/convex/convex/schema.ts
packages/*/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Place TypeScript packages under
packages/*; the Convex backend is located inpackages/convex.
Files:
packages/convex/convex/productAccountAuth.tspackages/convex/test/productAccount.test.tspackages/convex/convex/productAccount.tspackages/convex/convex/schema.ts
packages/*/test/**/*.ts
⚙️ CodeRabbit configuration file
packages/*/test/**/*.ts: Verify externally observable behavior, authorization and privacy
invariants, malformed inputs, and deterministic isolation. Flag tests that
duplicate the implementation or omit meaningful negative cases.
Files:
packages/convex/test/productAccount.test.ts
🔇 Additional comments (21)
README.md (1)
125-128: LGTM!packages/convex/convex/schema.ts (1)
1-17: LGTM!Also applies to: 64-64, 79-104
packages/convex/convex/productAccountAuth.ts (1)
6-16: LGTM!Also applies to: 68-68, 83-86
packages/convex/convex/productAccount.ts (2)
23-23: LGTM!Also applies to: 677-677, 700-702
1169-1186: 🩺 Stability & AvailabilityNo change needed.
commitPendingProductSyncKeyRotationis account-scoped and does not load or authorizetrustedDeviceId; the deleted ID is stored only as payload metadata.> Likely an incorrect or invalid review comment.apps/unwired-mail/unwired-mail/Services/Convex/ConvexClient.swift (2)
145-191: LGTM!Also applies to: 677-688
442-501: 🗄️ Data Integrity & IntegrationUse the device-scoped Product Sync queries. All three exports and argument names match the Swift client definitions.
apps/unwired-mail/unwired-mail/Services/ProductAccount/ProductAccountSession.swift (1)
274-312: LGTM!Also applies to: 1369-1381
apps/unwired-mail/unwired-mail/Services/ProductAccount/ProductAccountService.swift (1)
400-432: 🗄️ Data Integrity & IntegrationNo transition-gap issue found.
applyingTransitionuses the transition’s current or retained legacy key, so it repairs a lagging device when possible and otherwise throwsdecryptionFailed. The backend commits only when every registered device matches the pending epoch.apps/unwired-mail/unwired-mailTests/GenericMailSetupServiceTests.swift (2)
108-152: Assert that the refreshed snapshot reaches authorization.The test still asserts only
connected. TheisSessionCurrentandisSyncSessionCurrentclosures constrain the flow indirectly, but no assertion proves thatrefreshedSessionreachedGenericMailSetupService.authorizeand Product Sync. Record the session observed byRecordingGenericSyncServiceand assert it equalsrefreshedSession.As per path instructions: "Verify externally observable behavior, privacy invariants, error paths, cancellation or restart behavior where relevant, and deterministic isolation."
90-106: LGTM!packages/convex/test/productAccount.test.ts (2)
26-44: LGTM!Also applies to: 85-154, 166-175
2014-2051: LGTM!Also applies to: 2053-2165, 2231-2288
apps/unwired-mail/unwired-mailTests/ConvexClientTests.swift (2)
657-699: LGTM!Also applies to: 953-995, 997-1050
844-882: LGTM!apps/unwired-mail/unwired-mailTests/MailboxConnectionAdapterTests.swift (2)
771-804: LGTM!Also applies to: 5719-5750
4759-4783: LGTM!Also applies to: 8287-8334
apps/unwired-mail/unwired-mailTests/ProductAccountSessionTests.swift (4)
947-987: LGTM!
2985-3015: LGTM!Also applies to: 3017-3053, 3090-3132, 3134-3173, 3175-3220, 3273-3318
3590-3604: LGTM!Also applies to: 4102-4151
3260-3267: 🩺 Stability & AvailabilityConfirm the foreground authentication contract. When
scenePhasebecomes active,AccountViewinvokes both revalidation paths. An expired or unverifiable token makesrevalidateTrustedDeviceAfterForegrounding()call interactivesignIn(), whilerevalidateProductAccountAfterForegrounding()uses non-interactiverestoreSession(). Confirm whether foregrounding may show the Apple authorization sheet, then align the implementation and call-count assertions.
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5f6ac05372
ℹ️ 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 review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
apps/unwired-mail/unwired-mail/Services/ProductAccount/ProductAccountService.swift (1)
691-732: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winRelease the recovery gate with
defer.
revokeDevicereleasesproductAccountRecoveryOperationGatein three separate branches. A futurereturnorthrowinside thedoblock can skip the release and deadlock later operations for that Product Account. Acquire the gate, then release it once in adeferblock.♻️ Proposed refactor
await productAccountRecoveryOperationGate.acquire( productAccountId: session.productAccountId ) + defer { + Task { + await productAccountRecoveryOperationGate.release( + productAccountId: session.productAccountId + ) + } + }A synchronous
defercannot await, so either keep the explicit releases or extract the body into a privateperformRevokeDevicemethod and wrap it exactly likeloadandreplaceRecoveryKeydo.🤖 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/ProductAccount/ProductAccountService.swift` around lines 691 - 732, Refactor revokeDevice to guarantee a single recovery-gate release using the same private async helper pattern as load and replaceRecoveryKey, since defer cannot await directly. Keep gate acquisition in revokeDevice, perform the existing recovery and revoke logic in the helper, and release productAccountRecoveryOperationGate exactly once in the surrounding async cleanup flow while preserving the trusted-device error mapping.apps/unwired-mail/unwired-mailTests/MessageCategorizationServiceTests.swift (1)
2475-2493: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winAssert Trusted Device scope on changed payload reads.
The transport mock accepts but discards
trustedDeviceId. The affected tests can pass ifMessageCategoryAssignmentSyncServiceomits or substitutes the session device ID, so they do not validate device-scoped Product Sync reads.
apps/unwired-mail/unwired-mailTests/MessageCategorizationServiceTests.swift#L2475-L2493: RecordtrustedDeviceIdfor single, prefix, and batch retrieval calls.apps/unwired-mail/unwired-mailTests/MessageCategorizationServiceTests.swift#L1049-L1049: Assert that the legacy-compatible batch read usessession.trustedDeviceId.apps/unwired-mail/unwired-mailTests/MessageCategorizationServiceTests.swift#L1053-L1104: Assert that the current-and-legacy batch read usessession.trustedDeviceId.apps/unwired-mail/unwired-mailTests/MessageCategorizationServiceTests.swift#L1151-L1151: Keep the requested-payload count assertion and add a Trusted Device ID assertion.As per path instructions, tests must verify privacy invariants. The PR objective requires Product Sync payload access to use the Trusted Device.
🤖 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-mailTests/MessageCategorizationServiceTests.swift` around lines 2475 - 2493, Update apps/unwired-mail/unwired-mailTests/MessageCategorizationServiceTests.swift:2475-2493 so the transport mock records trustedDeviceId for single, prefix, and batch Product Sync retrievals. At 1049 and 1053-1104, assert the legacy-compatible and current-and-legacy batch reads use session.trustedDeviceId; at 1151, retain the requested-payload count assertion and also verify the Trusted Device ID.Source: Path instructions
packages/convex/convex/productAccountDeletionData.ts (2)
16-20: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDerive the revocation material validator from one definition.
This union repeats the
revocationMaterialunion inpackages/convex/convex/schema.tsLines 29-35. Two copies of one wire contract can drift when a material kind is added. Export the validator from a shared module and use it in both the schema and this file.🤖 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 `@packages/convex/convex/productAccountDeletionData.ts` around lines 16 - 20, Extract the revocationMaterialValidator definition that currently duplicates the union in schema.ts and export it from a shared module instead. Update productAccountDeletionData.ts to import this shared validator rather than redefining it locally, and update schema.ts to import and use the same shared definition. This ensures a single source of truth for the revocation material kinds to prevent drift when the validation contract changes.
744-755: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winBound the deletion continuation retries.
The catch block retries forever.
attemptonly caps the backoff delay atdeletionContinuationMaxRetryMilliseconds.deleteNextBatchDatathrows'Apple authorization revocation required'wheneverphase !== 'deleting-data'(Lines 609-611), and that condition does not heal by itself. The scheduler then runs this mutation every five minutes for the lifetime of the deployment, and the log line repeats at the same rate.Add a maximum attempt count. When the count is exceeded, stop rescheduling and record a terminal state that an operator can find.
🛡️ Proposed fix
+const deletionContinuationMaxAttempts = 12; + ... } catch { const attempt = (args.attempt ?? 0) + 1; console.error('Product Account deletion batch failed', { attempt }); + if (attempt > deletionContinuationMaxAttempts) { + return null; + } await ctx.scheduler.runAfter(🤖 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 `@packages/convex/convex/productAccountDeletionData.ts` around lines 744 - 755, Add a maximum attempt count check in the catch block of the continueProductAccountDeletion function before scheduling the retry. When the attempt count exceeds the maximum threshold, do not call ctx.scheduler.runAfter; instead, record a terminal state in the product account record (such as marking it as failed or blocked) that an operator can find, so the infinite retry loop is broken and the deletion attempt halts at the maximum attempt limit.apps/unwired-mail/unwired-mailTests/MicrosoftGraphMailboxConnectionAdapterTests.swift (1)
3033-3038: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winAssert that route removal occurs.
The assertion only proves local status deletion.
clearAllcan skipremoveMicrosoftGraphPushRouteand still pass, so the test does not exercisePRODUCT_ACCOUNT_DELETEDhandling. Assert the recorded opaque connection ID.Proposed test assertion
XCTAssertEqual(try statusStore.loadAll(productAccountId: session.productAccountId), []) + XCTAssertEqual( + routeTransport.removedOpaqueConnectionIds, + ["opaque-connection-id"] + )As per path instructions, tests must verify privacy invariants.
🤖 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-mailTests/MicrosoftGraphMailboxConnectionAdapterTests.swift` around lines 3033 - 3038, Update the test around clearAll to also assert that the recorded opaque Microsoft Graph connection ID was passed to removeMicrosoftGraphPushRoute, verifying PRODUCT_ACCOUNT_DELETED route removal. Keep the existing statusStore deletion assertion and use the test’s existing route-removal recording mechanism.Source: Path instructions
apps/unwired-mail/unwired-mail/Services/Mailbox/EWSMailboxConnectionAdapter.swift (1)
1562-1585: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winRecovery failure aborts the whole prefetch pass.
loadBodytreats only anilresult fromrecoverProviderMessageas skippable. If the recovery lookup throws, the error propagates out ofprefetchand stops the remaining messages inmessages. A permanently deleted message produces exactly this outcome:SystemEWSClient.recoverMessageIdentitythrowsErrorItemNotFoundwhen the bounded stable-key search finds no match, as asserted bytestSystemClientPreservesItemNotFoundWhenStableKeySearchHasNoMatch. Prefetch is a best-effort cache warm-up, so one unrecoverable message should not cancel the pass.Treat an item-not-found recovery failure as a skip and keep cancellation propagation intact.
♻️ Proposed fix to skip unrecoverable messages
} catch let error as EWSServiceError where error.isItemNotFound { - guard let recovered = try await recoverProviderMessage(providerMessage) else { return nil } + let recovered: EWSProviderMessage? + do { + recovered = try await recoverProviderMessage(providerMessage) + } catch let recoveryError as EWSServiceError where recoveryError.isItemNotFound { + return nil + } + guard let recovered else { return nil } return ( try await client.loadMessageBody( itemId: recovered.itemId, authorization: authorization ), recovered ) }🤖 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/Mailbox/EWSMailboxConnectionAdapter.swift` around lines 1562 - 1585, Update loadBody’s item-not-found recovery branch to catch EWSServiceError.isItemNotFound from recoverProviderMessage and return nil, treating the message as unrecoverable. Preserve nil recovery handling and allow all other errors, including cancellation, to propagate so prefetch can continue only for permanently missing messages.
🤖 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/Gmail/MessageCategorizationService.swift`:
- Around line 687-699: Update the payload loop in loadFutureLearningSignals
around decryptedLearningSignal so decryption failures skip only the affected
payload, while cancellation errors are rethrown as in loadAssignments. Continue
processing subsequent payloads and preserve the existing sender deduplication
and ordering behavior for successfully decrypted signals.
- Around line 921-949: In the legacy payload handling branch, move the
encoder.encode(storedSignal) call out of the legacyPayloads loop and perform it
once before iteration. Reuse the resulting plaintext for each
material.encryptPayload call, while continuing to derive associatedData from
each legacyIdentifier.
In `@apps/unwired-mail/unwired-mailTests/MessageCategorizationServiceTests.swift`:
- Around line 1106-1112: Extend the test around saveUserOverride and the
replacement learning signal to inspect the emitted payload metadata and assert
that its encryption key version is 2. Keep the existing signal-selection and
payload-count assertions, and specifically verify the replacement signal rather
than an unrelated write.
In
`@apps/unwired-mail/unwired-mailTests/MicrosoftGraphMailboxConnectionAdapterTests.swift`:
- Around line 2509-2528: Update both test sites in
apps/unwired-mail/unwired-mailTests/MicrosoftGraphMailboxConnectionAdapterTests.swift:2444-2470
and :2509-2528 to persist eligible MicrosoftGraphPushStatus records (including
routeId "route-id" at :2444-2470 and a status within the renewal window with a
fixed clock at :2509-2528), record revalidation calls, and assert revalidation
fails before any provider access. Preserve the existing renewal assertions and
verify RecordingMicrosoftGraphClient received no provider request in both tests.
---
Outside diff comments:
In
`@apps/unwired-mail/unwired-mail/Services/Mailbox/EWSMailboxConnectionAdapter.swift`:
- Around line 1562-1585: Update loadBody’s item-not-found recovery branch to
catch EWSServiceError.isItemNotFound from recoverProviderMessage and return nil,
treating the message as unrecoverable. Preserve nil recovery handling and allow
all other errors, including cancellation, to propagate so prefetch can continue
only for permanently missing messages.
In
`@apps/unwired-mail/unwired-mail/Services/ProductAccount/ProductAccountService.swift`:
- Around line 691-732: Refactor revokeDevice to guarantee a single recovery-gate
release using the same private async helper pattern as load and
replaceRecoveryKey, since defer cannot await directly. Keep gate acquisition in
revokeDevice, perform the existing recovery and revoke logic in the helper, and
release productAccountRecoveryOperationGate exactly once in the surrounding
async cleanup flow while preserving the trusted-device error mapping.
In `@apps/unwired-mail/unwired-mailTests/MessageCategorizationServiceTests.swift`:
- Around line 2475-2493: Update
apps/unwired-mail/unwired-mailTests/MessageCategorizationServiceTests.swift:2475-2493
so the transport mock records trustedDeviceId for single, prefix, and batch
Product Sync retrievals. At 1049 and 1053-1104, assert the legacy-compatible and
current-and-legacy batch reads use session.trustedDeviceId; at 1151, retain the
requested-payload count assertion and also verify the Trusted Device ID.
In
`@apps/unwired-mail/unwired-mailTests/MicrosoftGraphMailboxConnectionAdapterTests.swift`:
- Around line 3033-3038: Update the test around clearAll to also assert that the
recorded opaque Microsoft Graph connection ID was passed to
removeMicrosoftGraphPushRoute, verifying PRODUCT_ACCOUNT_DELETED route removal.
Keep the existing statusStore deletion assertion and use the test’s existing
route-removal recording mechanism.
In `@packages/convex/convex/productAccountDeletionData.ts`:
- Around line 16-20: Extract the revocationMaterialValidator definition that
currently duplicates the union in schema.ts and export it from a shared module
instead. Update productAccountDeletionData.ts to import this shared validator
rather than redefining it locally, and update schema.ts to import and use the
same shared definition. This ensures a single source of truth for the revocation
material kinds to prevent drift when the validation contract changes.
- Around line 744-755: Add a maximum attempt count check in the catch block of
the continueProductAccountDeletion function before scheduling the retry. When
the attempt count exceeds the maximum threshold, do not call
ctx.scheduler.runAfter; instead, record a terminal state in the product account
record (such as marking it as failed or blocked) that an operator can find, so
the infinite retry loop is broken and the deletion attempt halts at the maximum
attempt limit.
🪄 Autofix (Beta)
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: 4e07f252-320e-40c7-a7f7-dc26aa5850fd
📒 Files selected for processing (33)
README.mdapps/unwired-mail/unwired-mail/Features/Account/AccountView.swiftapps/unwired-mail/unwired-mail/Features/Settings/SettingsScene.swiftapps/unwired-mail/unwired-mail/Services/Convex/ConvexClient.swiftapps/unwired-mail/unwired-mail/Services/Gmail/GmailPushRelayService.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-mail/Services/Mailbox/PendingProviderActionService.swiftapps/unwired-mail/unwired-mail/Services/MicrosoftGraph/MicrosoftGraphPushRelayService.swiftapps/unwired-mail/unwired-mail/Services/ProductAccount/ProductAccountService.swiftapps/unwired-mail/unwired-mail/Services/ProductAccount/ProductAccountSession.swiftapps/unwired-mail/unwired-mail/Services/ProductAccount/ProductAccountSessionStore.swiftapps/unwired-mail/unwired-mailTests/ConvexClientTests.swiftapps/unwired-mail/unwired-mailTests/EWSMailboxConnectionAdapterTests.swiftapps/unwired-mail/unwired-mailTests/GenericMailSetupServiceTests.swiftapps/unwired-mail/unwired-mailTests/GmailPushRelayServiceTests.swiftapps/unwired-mail/unwired-mailTests/MailboxConnectionAdapterTests.swiftapps/unwired-mail/unwired-mailTests/MessageCategorizationServiceTests.swiftapps/unwired-mail/unwired-mailTests/MicrosoftGraphMailboxConnectionAdapterTests.swiftapps/unwired-mail/unwired-mailTests/PendingProviderActionServiceTests.swiftapps/unwired-mail/unwired-mailTests/ProductAccountSessionTests.swiftapps/unwired-mail/unwired-mailTests/ProductSyncKeyMaterialStoreTests.swiftapps/unwired-mail/unwired-mailTests/SettingsDestinationRegistryTests.swiftpackages/convex/convex/productAccount.tspackages/convex/convex/productAccountAuth.tspackages/convex/convex/productAccountDeletionData.tspackages/convex/convex/productSync.tspackages/convex/convex/schema.tspackages/convex/test/productAccount.test.tspackages/convex/test/productSync.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Apple · Unwired Mail
🧰 Additional context used
📓 Path-based instructions (11)
**/*
📄 CodeRabbit inference engine (AGENTS.md)
**/*: Inspect nearby implementation, tests, and pattern documentation before editing, and prefer existing abstractions and conventions.
Every change must consider documentation and update relevant documentation when behavior, setup, commands, architecture, environment variables, public interfaces, or agent workflow expectations change.
When addressing GitHub comments after fixing them and pushing PR changes, resolve the comments and reference the issue the PR solves.
Before finishing, confirm only intended files changed, run relevant validation, update required documentation, and summarize verified and unrun commands.
Files:
apps/unwired-mail/unwired-mail/Services/ProductAccount/ProductAccountSessionStore.swiftapps/unwired-mail/unwired-mailTests/SettingsDestinationRegistryTests.swiftpackages/convex/convex/productAccountDeletionData.tsapps/unwired-mail/unwired-mailTests/GmailPushRelayServiceTests.swiftapps/unwired-mail/unwired-mailTests/PendingProviderActionServiceTests.swiftapps/unwired-mail/unwired-mailTests/MicrosoftGraphMailboxConnectionAdapterTests.swiftREADME.mdapps/unwired-mail/unwired-mail/Services/Mailbox/IMAPMailboxConnectionAdapter.swiftapps/unwired-mail/unwired-mail/Services/Convex/ConvexClient.swiftapps/unwired-mail/unwired-mailTests/EWSMailboxConnectionAdapterTests.swiftapps/unwired-mail/unwired-mailTests/MessageCategorizationServiceTests.swiftapps/unwired-mail/unwired-mail/Services/Mailbox/PendingProviderActionService.swiftapps/unwired-mail/unwired-mailTests/GenericMailSetupServiceTests.swiftapps/unwired-mail/unwired-mailTests/ProductSyncKeyMaterialStoreTests.swiftapps/unwired-mail/unwired-mail/Services/Mailbox/EWSMailboxConnectionAdapter.swiftapps/unwired-mail/unwired-mailTests/MailboxConnectionAdapterTests.swiftapps/unwired-mail/unwired-mail/Services/MicrosoftGraph/MicrosoftGraphPushRelayService.swiftapps/unwired-mail/unwired-mailTests/ConvexClientTests.swiftpackages/convex/test/productAccount.test.tsapps/unwired-mail/unwired-mail/Services/Gmail/MessageCategorizationService.swiftapps/unwired-mail/unwired-mail/Services/Mailbox/MailboxConnection.swiftpackages/convex/convex/productAccountAuth.tspackages/convex/test/productSync.test.tspackages/convex/convex/productSync.tspackages/convex/convex/schema.tspackages/convex/convex/productAccount.tsapps/unwired-mail/unwired-mail/Services/ProductAccount/ProductAccountSession.swiftapps/unwired-mail/unwired-mail/Services/ProductAccount/ProductAccountService.swiftapps/unwired-mail/unwired-mailTests/ProductAccountSessionTests.swiftapps/unwired-mail/unwired-mail/Services/Mailbox/MicrosoftGraphMailboxConnectionAdapter.swiftapps/unwired-mail/unwired-mail/Services/Gmail/GmailPushRelayService.swiftapps/unwired-mail/unwired-mail/Features/Settings/SettingsScene.swiftapps/unwired-mail/unwired-mail/Features/Account/AccountView.swift
apps/unwired-mail/**/*.{swift,plist}
📄 CodeRabbit inference engine (AGENTS.md)
apps/unwired-mail/**/*.{swift,plist}: Apple app code belongs underapps/unwired-mailand must provide formatter, linter, and test commands.
For Apple validation, runzsh scripts/check-apple-lint.zshfor formatting and linting, and run the specifiedxcodebuild testcommand for theunwired-mailiOS scheme.
Files:
apps/unwired-mail/unwired-mail/Services/ProductAccount/ProductAccountSessionStore.swiftapps/unwired-mail/unwired-mailTests/SettingsDestinationRegistryTests.swiftapps/unwired-mail/unwired-mailTests/GmailPushRelayServiceTests.swiftapps/unwired-mail/unwired-mailTests/PendingProviderActionServiceTests.swiftapps/unwired-mail/unwired-mailTests/MicrosoftGraphMailboxConnectionAdapterTests.swiftapps/unwired-mail/unwired-mail/Services/Mailbox/IMAPMailboxConnectionAdapter.swiftapps/unwired-mail/unwired-mail/Services/Convex/ConvexClient.swiftapps/unwired-mail/unwired-mailTests/EWSMailboxConnectionAdapterTests.swiftapps/unwired-mail/unwired-mailTests/MessageCategorizationServiceTests.swiftapps/unwired-mail/unwired-mail/Services/Mailbox/PendingProviderActionService.swiftapps/unwired-mail/unwired-mailTests/GenericMailSetupServiceTests.swiftapps/unwired-mail/unwired-mailTests/ProductSyncKeyMaterialStoreTests.swiftapps/unwired-mail/unwired-mail/Services/Mailbox/EWSMailboxConnectionAdapter.swiftapps/unwired-mail/unwired-mailTests/MailboxConnectionAdapterTests.swiftapps/unwired-mail/unwired-mail/Services/MicrosoftGraph/MicrosoftGraphPushRelayService.swiftapps/unwired-mail/unwired-mailTests/ConvexClientTests.swiftapps/unwired-mail/unwired-mail/Services/Gmail/MessageCategorizationService.swiftapps/unwired-mail/unwired-mail/Services/Mailbox/MailboxConnection.swiftapps/unwired-mail/unwired-mail/Services/ProductAccount/ProductAccountSession.swiftapps/unwired-mail/unwired-mail/Services/ProductAccount/ProductAccountService.swiftapps/unwired-mail/unwired-mailTests/ProductAccountSessionTests.swiftapps/unwired-mail/unwired-mail/Services/Mailbox/MicrosoftGraphMailboxConnectionAdapter.swiftapps/unwired-mail/unwired-mail/Services/Gmail/GmailPushRelayService.swiftapps/unwired-mail/unwired-mail/Features/Settings/SettingsScene.swiftapps/unwired-mail/unwired-mail/Features/Account/AccountView.swift
**/*.{ts,tsx,swift}
📄 CodeRabbit inference engine (AGENTS.md)
Every code change must have appropriate lint, format, and test coverage before handoff unless a required tool or platform dependency is unavailable; start with the smallest meaningful checks and broaden for shared or cross-package changes.
Files:
apps/unwired-mail/unwired-mail/Services/ProductAccount/ProductAccountSessionStore.swiftapps/unwired-mail/unwired-mailTests/SettingsDestinationRegistryTests.swiftpackages/convex/convex/productAccountDeletionData.tsapps/unwired-mail/unwired-mailTests/GmailPushRelayServiceTests.swiftapps/unwired-mail/unwired-mailTests/PendingProviderActionServiceTests.swiftapps/unwired-mail/unwired-mailTests/MicrosoftGraphMailboxConnectionAdapterTests.swiftapps/unwired-mail/unwired-mail/Services/Mailbox/IMAPMailboxConnectionAdapter.swiftapps/unwired-mail/unwired-mail/Services/Convex/ConvexClient.swiftapps/unwired-mail/unwired-mailTests/EWSMailboxConnectionAdapterTests.swiftapps/unwired-mail/unwired-mailTests/MessageCategorizationServiceTests.swiftapps/unwired-mail/unwired-mail/Services/Mailbox/PendingProviderActionService.swiftapps/unwired-mail/unwired-mailTests/GenericMailSetupServiceTests.swiftapps/unwired-mail/unwired-mailTests/ProductSyncKeyMaterialStoreTests.swiftapps/unwired-mail/unwired-mail/Services/Mailbox/EWSMailboxConnectionAdapter.swiftapps/unwired-mail/unwired-mailTests/MailboxConnectionAdapterTests.swiftapps/unwired-mail/unwired-mail/Services/MicrosoftGraph/MicrosoftGraphPushRelayService.swiftapps/unwired-mail/unwired-mailTests/ConvexClientTests.swiftpackages/convex/test/productAccount.test.tsapps/unwired-mail/unwired-mail/Services/Gmail/MessageCategorizationService.swiftapps/unwired-mail/unwired-mail/Services/Mailbox/MailboxConnection.swiftpackages/convex/convex/productAccountAuth.tspackages/convex/test/productSync.test.tspackages/convex/convex/productSync.tspackages/convex/convex/schema.tspackages/convex/convex/productAccount.tsapps/unwired-mail/unwired-mail/Services/ProductAccount/ProductAccountSession.swiftapps/unwired-mail/unwired-mail/Services/ProductAccount/ProductAccountService.swiftapps/unwired-mail/unwired-mailTests/ProductAccountSessionTests.swiftapps/unwired-mail/unwired-mail/Services/Mailbox/MicrosoftGraphMailboxConnectionAdapter.swiftapps/unwired-mail/unwired-mail/Services/Gmail/GmailPushRelayService.swiftapps/unwired-mail/unwired-mail/Features/Settings/SettingsScene.swiftapps/unwired-mail/unwired-mail/Features/Account/AccountView.swift
apps/unwired-mail/**/*.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/ProductAccount/ProductAccountSessionStore.swiftapps/unwired-mail/unwired-mailTests/SettingsDestinationRegistryTests.swiftapps/unwired-mail/unwired-mailTests/GmailPushRelayServiceTests.swiftapps/unwired-mail/unwired-mailTests/PendingProviderActionServiceTests.swiftapps/unwired-mail/unwired-mailTests/MicrosoftGraphMailboxConnectionAdapterTests.swiftapps/unwired-mail/unwired-mail/Services/Mailbox/IMAPMailboxConnectionAdapter.swiftapps/unwired-mail/unwired-mail/Services/Convex/ConvexClient.swiftapps/unwired-mail/unwired-mailTests/EWSMailboxConnectionAdapterTests.swiftapps/unwired-mail/unwired-mailTests/MessageCategorizationServiceTests.swiftapps/unwired-mail/unwired-mail/Services/Mailbox/PendingProviderActionService.swiftapps/unwired-mail/unwired-mailTests/GenericMailSetupServiceTests.swiftapps/unwired-mail/unwired-mailTests/ProductSyncKeyMaterialStoreTests.swiftapps/unwired-mail/unwired-mail/Services/Mailbox/EWSMailboxConnectionAdapter.swiftapps/unwired-mail/unwired-mailTests/MailboxConnectionAdapterTests.swiftapps/unwired-mail/unwired-mail/Services/MicrosoftGraph/MicrosoftGraphPushRelayService.swiftapps/unwired-mail/unwired-mailTests/ConvexClientTests.swiftapps/unwired-mail/unwired-mail/Services/Gmail/MessageCategorizationService.swiftapps/unwired-mail/unwired-mail/Services/Mailbox/MailboxConnection.swiftapps/unwired-mail/unwired-mail/Services/ProductAccount/ProductAccountSession.swiftapps/unwired-mail/unwired-mail/Services/ProductAccount/ProductAccountService.swiftapps/unwired-mail/unwired-mailTests/ProductAccountSessionTests.swiftapps/unwired-mail/unwired-mail/Services/Mailbox/MicrosoftGraphMailboxConnectionAdapter.swiftapps/unwired-mail/unwired-mail/Services/Gmail/GmailPushRelayService.swiftapps/unwired-mail/unwired-mail/Features/Settings/SettingsScene.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/SettingsDestinationRegistryTests.swiftapps/unwired-mail/unwired-mailTests/GmailPushRelayServiceTests.swiftapps/unwired-mail/unwired-mailTests/PendingProviderActionServiceTests.swiftapps/unwired-mail/unwired-mailTests/MicrosoftGraphMailboxConnectionAdapterTests.swiftapps/unwired-mail/unwired-mailTests/EWSMailboxConnectionAdapterTests.swiftapps/unwired-mail/unwired-mailTests/MessageCategorizationServiceTests.swiftapps/unwired-mail/unwired-mailTests/GenericMailSetupServiceTests.swiftapps/unwired-mail/unwired-mailTests/ProductSyncKeyMaterialStoreTests.swiftapps/unwired-mail/unwired-mailTests/MailboxConnectionAdapterTests.swiftapps/unwired-mail/unwired-mailTests/ConvexClientTests.swiftapps/unwired-mail/unwired-mailTests/ProductAccountSessionTests.swift
packages/convex/convex/**/*.ts
📄 CodeRabbit inference engine (packages/convex/CLAUDE.md)
For work on Convex code, always read
convex/_generated/ai/guidelines.mdfirst; its rules override general Convex knowledge when using Convex APIs and patterns.For Convex code, always read
convex/_generated/ai/guidelines.mdfirst; its rules override general Convex knowledge when using Convex APIs and patterns.
Files:
packages/convex/convex/productAccountDeletionData.tspackages/convex/convex/productAccountAuth.tspackages/convex/convex/productSync.tspackages/convex/convex/schema.tspackages/convex/convex/productAccount.ts
⚙️ CodeRabbit configuration file
packages/convex/convex/**/*.ts: Treat authentication, authorization, input validation, and privacy
boundaries as mandatory. Flag public functions that do not validate
arguments and returns, data access that is not scoped to the authenticated
Product Account or trusted device, plaintext sensitive data, secrets in
logs, unbounded reads, and queries that bypass appropriate indexes.
Changes to runtime behavior should include focused Convex tests covering
success, unauthorized access, malformed input, and relevant retries.
Files:
packages/convex/convex/productAccountDeletionData.tspackages/convex/convex/productAccountAuth.tspackages/convex/convex/productSync.tspackages/convex/convex/schema.tspackages/convex/convex/productAccount.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use pnpm and the repository TypeScript tooling: Oxlint with
@rajzik/oxlint-config, Oxfmt with@rajzik/oxfmt-config, and TypeScript configuration extending@rajzik/tsconfig.
Files:
packages/convex/convex/productAccountDeletionData.tspackages/convex/test/productAccount.test.tspackages/convex/convex/productAccountAuth.tspackages/convex/test/productSync.test.tspackages/convex/convex/productSync.tspackages/convex/convex/schema.tspackages/convex/convex/productAccount.ts
packages/*/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Place TypeScript packages under
packages/*; the Convex backend is located inpackages/convex.
Files:
packages/convex/convex/productAccountDeletionData.tspackages/convex/test/productAccount.test.tspackages/convex/convex/productAccountAuth.tspackages/convex/test/productSync.test.tspackages/convex/convex/productSync.tspackages/convex/convex/schema.tspackages/convex/convex/productAccount.ts
README.md
📄 CodeRabbit inference engine (AGENTS.md)
Use
README.mdfor developer setup, common commands, and high-level project navigation.
Files:
README.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:
README.md
packages/*/test/**/*.ts
⚙️ CodeRabbit configuration file
packages/*/test/**/*.ts: Verify externally observable behavior, authorization and privacy
invariants, malformed inputs, and deterministic isolation. Flag tests that
duplicate the implementation or omit meaningful negative cases.
Files:
packages/convex/test/productAccount.test.tspackages/convex/test/productSync.test.ts
🧠 Learnings (1)
📚 Learning: 2026-07-27T07:25:56.352Z
Learnt from: rajzik
Repo: unwired-dev/product PR: 201
File: apps/unwired-mail/unwired-mail/Services/Mailbox/OutboxDeliveryService.swift:0-0
Timestamp: 2026-07-27T07:25:56.352Z
Learning: In the EWS mail provider, keep the failure-disposition/classification logic intentionally different between `OutboxDeliveryService` and `PendingProviderActionService`.
- `OutboxDeliveryService` should treat uncertain sends as “ambiguous” (classify HTTP 5xx, HTTP 408/409/425, `ErrorTimeoutExpired`, and invalid/failed EWS responses as ambiguous) and reconcile before allowing another send to prevent duplicates.
- `PendingProviderActionService` should treat its retry-safe EWS response cases as transient so they trigger provider actions as retries.
During review, do not refactor these into a shared helper solely because the response-code checks overlap—only extract if you can prove the semantics (ambiguous-before-retry vs retry-safe transient) remain unchanged for each service.
Applied to files:
apps/unwired-mail/unwired-mail/Services/Mailbox/PendingProviderActionService.swift
🔇 Additional comments (54)
packages/convex/convex/productAccount.ts (1)
677-677: LGTM!Also applies to: 688-712
packages/convex/convex/schema.ts (2)
9-17: LGTM!Also applies to: 64-64, 79-103
29-35: 🔒 Security & PrivacyConfirm the retention boundary for stored Apple credentials.
Document when
revocationMaterialis cleared or deleted, and link the ADR or privacy document that covers these at-rest credentials.packages/convex/convex/productAccountAuth.ts (1)
6-43: LGTM!Also applies to: 68-68, 83-86
packages/convex/convex/productAccountDeletionData.ts (2)
22-45: LGTM!Also applies to: 72-203, 550-598, 601-715
47-70: 🩺 Stability & AvailabilityVerify recovery-chain deduplication. Confirm that all paths scheduling
scheduleRevocationRecoveryuserevocationRecoveryScheduledAtto prevent duplicate chains.packages/convex/test/productAccount.test.ts (2)
26-44: LGTM!Also applies to: 85-174, 2014-2066, 2068-2180, 2182-2344, 2399-2662, 2664-3049
2355-2382: 🔒 Security & PrivacyAdd cross-account coverage for deletion request mutations.
If
ownedDeletionRequestcompares the requesttokenIdentifierwith the caller identity, prepare a deletion request as one Apple identity, callstoreRevocationTokenas another identity, and expect'Product Account deletion request required'. Apply the same coverage to the other deletion request mutations.packages/convex/convex/productSync.ts (2)
206-227: LGTM!Also applies to: 341-347, 368-381, 388-398, 423-436, 443-453
311-334: 📐 Maintainability & Code QualitySince the repository is private and cannot be accessed for verification, manual inspection is required to confirm that no remaining callers use the account-scoped legacy queries before adding deprecation comments.
apps/unwired-mail/unwired-mail/Services/Convex/ConvexClient.swift (1)
21-24: LGTM!Also applies to: 145-206, 442-491, 536-545, 628-628, 671-688, 731-735
apps/unwired-mail/unwired-mailTests/ConvexClientTests.swift (1)
149-182: LGTM!Also applies to: 657-699, 953-995, 997-1050
apps/unwired-mail/unwired-mailTests/ProductSyncKeyMaterialStoreTests.swift (1)
104-346: LGTM!Also applies to: 388-667, 695-722, 1269-1356, 1541-1598
apps/unwired-mail/unwired-mail/Services/ProductAccount/ProductAccountService.swift (2)
226-255: LGTM!Also applies to: 275-349, 793-846
257-273: 🔒 Security & PrivacyThe repository requires authentication to access, and I cannot verify the claims in this review comment. I am unable to:
- Confirm whether
translatingTrustedDeviceRevocationexists and is used by otherConvexProductAccountServicemethods.- Verify that
deleteProductAccountmaps onlyPRODUCT_ACCOUNT_DELETEDand notTRUSTED_DEVICE_REVOKED.- Inspect the backend code to confirm that
TRUSTED_DEVICE_REVOKEDcan be raised during deletion.- Examine the
ProductAccountSession.performProductAccountDeletionflow.The review comment's claims about error handling and security implications cannot be assessed without access to the codebase.
apps/unwired-mail/unwired-mail/Services/ProductAccount/ProductAccountSession.swift (1)
274-329: LGTM!Also applies to: 363-390, 496-541, 572-580, 1253-1298, 1333-1381
apps/unwired-mail/unwired-mail/Services/ProductAccount/ProductAccountSessionStore.swift (1)
66-68: LGTM!Also applies to: 250-271, 377-387
README.md (1)
125-128: LGTM!apps/unwired-mail/unwired-mailTests/ProductAccountSessionTests.swift (1)
947-987: LGTM!Also applies to: 2985-3053, 3055-3088, 3090-3173, 3175-3220, 3222-3318, 4030-4062, 4149-4198
apps/unwired-mail/unwired-mail/Features/Account/AccountView.swift (2)
787-804: LGTM!Also applies to: 1105-1152, 1246-1257, 1412-1475, 1630-1645, 1812-1845, 1881-1897, 1914-1997, 3213-3373, 3536-3565, 5167-5213, 5262-5309, 5569-5611
346-347: 🎯 Functional CorrectnessVerify all
MailboxFreshnessViewModelconstruction sites supplyisSessionIdentityCurrent. If a factory omits this closure,pollWhileActivefalls back to the token-sensitiveisSessionCurrentcheck and can stop after an identity-token refresh.apps/unwired-mail/unwired-mail/Services/MicrosoftGraph/MicrosoftGraphPushRelayService.swift (1)
617-622: LGTM!Also applies to: 686-750, 753-835
apps/unwired-mail/unwired-mail/Services/Gmail/GmailPushRelayService.swift (2)
1241-1307: LGTM!Also applies to: 1328-1328, 1352-1404, 2016-2064
359-376: 🎯 Functional CorrectnessConfirm that
clearGmailPushNotificationState(productAccountId:providerAccountIdentifier:defaults:)exists. The test calls this three-parameter overload, but the shown declaration has only two parameters.apps/unwired-mail/unwired-mailTests/GenericMailSetupServiceTests.swift (1)
90-154: LGTM!Also applies to: 2389-2389, 2486-2505
apps/unwired-mail/unwired-mailTests/GmailPushRelayServiceTests.swift (1)
10-31: LGTM!Also applies to: 3651-3679, 3681-3697, 3835-3852, 3884-3886
apps/unwired-mail/unwired-mailTests/MailboxConnectionAdapterTests.swift (1)
753-805: LGTM!Also applies to: 5720-5751
apps/unwired-mail/unwired-mailTests/SettingsDestinationRegistryTests.swift (1)
786-807: LGTM!apps/unwired-mail/unwired-mailTests/MicrosoftGraphMailboxConnectionAdapterTests.swift (1)
3528-3528: LGTM!Also applies to: 3574-3574
apps/unwired-mail/unwired-mail/Services/Mailbox/EWSMailboxConnectionAdapter.swift (5)
245-255: LGTM!Also applies to: 312-325
2237-2259: LGTM!Also applies to: 2299-2330
2449-2542: LGTM!Also applies to: 3147-3176
3011-3110: LGTM!
2685-2802: LGTM!apps/unwired-mail/unwired-mail/Services/Mailbox/IMAPMailboxConnectionAdapter.swift (1)
2428-2444: LGTM!Also applies to: 2458-2471
apps/unwired-mail/unwired-mail/Services/Mailbox/MailboxConnection.swift (2)
1704-1711: LGTM!Also applies to: 1727-1734
3056-3115: LGTM!Also applies to: 3244-3272, 3303-3318
apps/unwired-mail/unwired-mail/Services/Mailbox/MicrosoftGraphMailboxConnectionAdapter.swift (1)
3332-3352: LGTM!Also applies to: 3368-3386, 3404-3421, 3550-3551, 3565-3565
apps/unwired-mail/unwired-mail/Services/Mailbox/PendingProviderActionService.swift (2)
395-412: LGTM!Also applies to: 414-420, 434-439
442-463: 🔒 Security & PrivacyConfirm that
discardBlockedActionandwaitForPendingActionRetriesenforcerevalidateProviderAccessbefore provider mutations. The available source does not establish whether these paths execute unfenced actions.apps/unwired-mail/unwired-mailTests/EWSMailboxConnectionAdapterTests.swift (5)
82-97: LGTM!
1704-1766: LGTM!Also applies to: 2755-2860
3475-3547: LGTM!Also applies to: 3549-3671
4014-4105: LGTM!Also applies to: 4107-4191, 4193-4282
5337-5346: LGTM!Also applies to: 5358-5380, 5440-5513, 5557-5557
apps/unwired-mail/unwired-mailTests/PendingProviderActionServiceTests.swift (1)
1144-1183: LGTM!apps/unwired-mail/unwired-mail/Features/Settings/SettingsScene.swift (4)
1168-1173: LGTM!Also applies to: 1243-1243, 1499-1522
1253-1261: LGTM!Also applies to: 1283-1307, 1336-1346, 1355-1387, 1395-1406, 1448-1460
1181-1181: LGTM!Also applies to: 1209-1227, 1279-1280, 1425-1440
1557-1565: LGTM!apps/unwired-mail/unwired-mail/Services/Gmail/MessageCategorizationService.swift (2)
601-605: LGTM!Also applies to: 637-641, 773-777, 989-1006
877-903: 🗄️ Data Integrity & IntegrationVerify that legacy-identifier payloads decrypt with the current key material.
Line 901 decrypts every legacy-key payload through
material.decryptPayload, using the payload's own identifier as associated data. Confirm thatdecryptPayloaditerates legacy epochs when matching the decryption key, and confirm the behavior when a legacy epoch has been pruned. A failure here throws out ofsaveLearningSignaland blocks the user-override write.packages/convex/test/productSync.test.ts (2)
118-139: LGTM!
141-156: 🎯 Functional CorrectnessProvide repository access to determine whether this test should call
revokeTrustedDevice.
|
@coderabbitai review |
✅ Action performedReview finished.
|
1 participant
Closes #129
Summary
Validation
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.