Skip to content

Add encrypted Mail Profile foundation - #448

Merged
rajzik merged 3 commits into
mainfrom
codex/issue-390-mail-profile
Aug 11, 2026
Merged

rajzik merged 3 commits into
mainfrom
codex/issue-390-mail-profile

Conversation

@gipity-bot

@gipity-bot gipity-bot Bot commented Aug 11, 2026 •

Copy link
Copy Markdown
Contributor

Closes #390

Summary

  • add encrypted Mail Profile definitions, explicit Mailbox Connection ownership, and opaque record scopes
  • migrate existing Product Accounts losslessly into one deterministic Default Profile while retaining all deployed Product Account-scoped record identifiers
  • isolate Profile ownership from legacy connection payloads so older clients cannot erase it, and preserve same-field conflicts without wall-clock last-writer-wins
  • document the privacy and mixed-version boundary and add the required package changeset

Validation

  • mise exec -- pnpm install --frozen-lockfile
  • mise exec -- pnpm lint
  • mise exec -- pnpm format
  • mise exec -- pnpm turbo run check-types
  • mise exec -- pnpm test
  • mise exec -- pnpm fallow (zero issues)
  • mise exec -- zsh scripts/check-apple-lint.zsh (122 files, zero violations)
  • focused MailboxConnectionSyncServiceTests on iPhone 17 Simulator UDID D6D43A14-3B1A-41B6-B25A-0DFCA1BFB089 (37 tests passed)
  • broad Debug Apple suite on the same UDID, excluding the two isolated CI fixtures (1,789 tests passed; one existing known issue)
  • isolated mixed-connection scenario on the same UDID (passed)
  • Release-only cached-presentation performance fixture on the same UDID (passed)

Remaining human actions

  • Normal code review only; no manual prerequisite or unavailable validation remains for this issue-scoped foundation.

Profile switching, lifecycle UI, lock behavior, and complete Settings integration remain tracked by the existing child issues and are intentionally outside #390.


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

@coderabbitai

coderabbitai Bot commented Aug 11, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 6 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 00109886-d76d-4d71-a4c0-5ab117e98ecd

📥 Commits

Reviewing files that changed from the base of the PR and between 51bbb63 and 7781d27.

📒 Files selected for processing (5)
  • CONTEXT.md
  • apps/unwired-mail/unwired-mail/Services/ProductSync/MailboxConnectionSyncModels.swift
  • apps/unwired-mail/unwired-mail/Services/ProductSync/MailboxConnectionSyncService.swift
  • apps/unwired-mail/unwired-mailTests/MailboxConnectionSyncServiceTests.swift
  • docs/adr/0048-separate-encrypted-mail-profile-ownership.md
📝 Walkthrough

Summary

  • Adds encrypted Mail Profile ownership in the Apple Product Sync layer.
  • Migrates existing Product Accounts into one deterministic Default Profile without changing deployed record identifiers.
  • Assigns Mailbox Connections to explicit Profile scopes.
  • Keeps provider credentials device-local in the Keychain.
  • Prevents legacy connection payloads from overwriting Profile ownership.
  • Adds field-level merge handling and explicit conflict copies.

Migration and privacy

  • Reconciles legacy connections idempotently.
  • Removes invalid assignments and assigns unscoped active connections to the Default Profile.
  • Fails closed for unknown Profile scopes.
  • Documents mixed-version behavior, rollback, and deferred UI work.
  • Adds ADR 0048, domain definitions, and a release changeset.

Affected areas

  • Apple: MailboxConnectionSyncModels.swift, MailboxConnectionSyncService.swift, and MailboxConnectionSyncServiceTests.swift.
  • TypeScript: No TypeScript changes are included.

Validation

  • Covers migration, idempotent assignment, scoped queries, missing Profiles, offline field merges, conflict resolution, and opaque sync scopes.
  • Includes linting, formatting, type checks, Apple linting, focused and broad Apple tests, mixed-connection tests, and the Release performance fixture.

Walkthrough

The PR adds encrypted Mail Profile models and Product Sync storage. It migrates legacy mailbox connections into a deterministic Default Profile, adds profile-scoped queries and edits, supports field-level conflict handling, and documents and tests the behavior.

Changes

Mail Profile foundation

Layer / File(s) Summary
Profile contracts and legacy migration
apps/unwired-mail/unwired-mail/Services/ProductSync/MailboxConnectionSyncModels.swift, CONTEXT.md, docs/adr/...
Defines profile identities, scopes, assignments, migration rules, validation errors, and encrypted ownership boundaries.
Profile service and authoritative storage
apps/unwired-mail/unwired-mail/Services/ProductSync/MailboxConnectionSyncService.swift
Adds profile persistence, scoped connection loading, profile creation, field-level merging, conflict copies, validation, and conflict resolution.
Profile behavior and transport tests
apps/unwired-mail/unwired-mailTests/MailboxConnectionSyncServiceTests.swift
Tests migration, idempotent assignment, scoped queries, conflict handling, opaque scopes, and encrypted payload access.
Release metadata
.changeset/encrypted-mail-profile-foundation.md
Declares a patch release for unwired-mail.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant MailboxConnectionSyncService
  participant ProductSync
  participant EncryptedProfileRecord
  Client->>MailboxConnectionSyncService: loadProfileSnapshot(session)
  MailboxConnectionSyncService->>ProductSync: load active connections
  ProductSync-->>MailboxConnectionSyncService: return connection snapshot
  MailboxConnectionSyncService->>EncryptedProfileRecord: load and migrate profile payload
  EncryptedProfileRecord-->>MailboxConnectionSyncService: return synchronized profiles
  MailboxConnectionSyncService-->>Client: return profile snapshot and scoped connections
Loading

Possibly related issues

  • unwired-dev/product#389 — Defines the broader synchronized Mail Profile and lossless migration scope implemented by this foundation.
  • unwired-dev/product#156 — Extends the profile-scoped ownership boundary to Mail Views and related mail state.
  • unwired-dev/product#392 — Extends the profile assignment model with profile duplication, transfer, and deletion lifecycle operations.

Possibly related PRs

Suggested reviewers: rajzik

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The PR addresses profile sync, migration, scoping, conflicts, and documentation, but does not evidence all product-owned state or rollback coverage required by [#390]. Implement or explicitly defer the missing product-owned state boundaries and add focused rollback coverage before closing [#390].
✅ Passed checks (4 passed)
Check name Status Explanation
Out of Scope Changes check ✅ Passed The code, tests, documentation, changeset, and validation directly support the encrypted Mail Profile foundation described in [#390].
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Title check ✅ Passed The title clearly and concisely identifies the main change: adding the encrypted Mail Profile foundation.
Description check ✅ Passed The description directly explains the Mail Profile changes, migration, conflict handling, documentation, and validation.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/issue-390-mail-profile

Comment @coderabbitai help to get the list of available commands.

@rajzik
rajzik marked this pull request as ready for review August 11, 2026 14:45
@github-actions

Copy link
Copy Markdown

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Aug 11, 2026 •

Copy link
Copy Markdown
Contributor
⚠️ Action not completed

Already reviewed.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 8

🤖 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/ProductSync/MailboxConnectionSyncModels.swift`:
- Around line 301-319: The reconciliation logic around activeIds and
retainedAssignments must preserve assignments for connections absent from the
current payload, removing them only when an explicit connection-removal
tombstone exists; continue assigning default profiles to newly active,
unassigned connections. In
apps/unwired-mail/unwired-mail/Services/ProductSync/MailboxConnectionSyncModels.swift#L301-L319,
update the filtering logic accordingly. In
docs/adr/0048-separate-encrypted-mail-profile-ownership.md#L17-L17, document
this retention rule in the Decision and the resulting ownership consequence in
Consequences.
- Around line 285-300: Update the profile-repair logic before the guard in the
validation flow so a non-nil defaultProfileId that is absent from profiles
re-adds defaultProfile, resets defaultProfileId to defaultProfile.id, and marks
changed. Keep the repair deterministic and idempotent, then allow the existing
guard to validate the repaired profile set.

In
`@apps/unwired-mail/unwired-mail/Services/ProductSync/MailboxConnectionSyncService.swift`:
- Around line 568-574: Update the profile-name validation in createProfile and
validateProfilePayload to reject duplicate names and names exceeding 40
characters with a dedicated MailProfileSyncError case, rather than
concurrentModification or generic invalidProfileState. Ensure the new error
communicates invalid or duplicate input and is used consistently for both
validation paths.
- Around line 774-786: Update MailProfileFieldValue.set(_:for:) to return Bool,
returning true only when the field and value cases match and are assigned, and
false for mismatches. In resolveProfileConflict, check this result before
removing the conflict or persisting the profile, and reject the resolution when
it is false so mismatched synchronized payloads cannot be silently discarded.
- Around line 681-694: Update updateProfiles so
Self.validateProfilePayload(payload) runs only when mutation returns true and
the payload will be written; allow the changed == false read path to return
.acceptAuthoritative without validating legacy or mixed-version values. Preserve
sorting and resolvedPayload assignment for both paths.

In `@apps/unwired-mail/unwired-mailTests/MailboxConnectionSyncServiceTests.swift`:
- Around line 1127-1149: Add tests alongside
testDefaultProfileMigrationIsIdempotentAndClaimsConnectionsAddedByOlderClients
covering three gaps: retain a non-default Profile assignment when its connection
is removed from and later restored to the payload; accept and return a seeded
mail-profiles-primary payload containing a Profile name longer than 40
characters; and verify a Profile operation throws
MailProfileSyncError.missingProductSyncKeyMaterial when key material is absent
while another product-sync payload exists. Reuse the existing services, payload,
and error-test setup patterns.

In `@CONTEXT.md`:
- Around line 395-406: Add relationship bullets in the Relationships section
covering that each Mailbox Connection belongs to exactly one Mail Profile,
Profile-scoped queries require an explicit Mail Profile, and provider
credentials remain device-local and outside Profile Record Scope. Align the
terminology with Mail Profile, Mailbox Connection, and Profile Record Scope as
defined in the diff and ADR 0048.

In `@docs/adr/0048-separate-encrypted-mail-profile-ownership.md`:
- Line 17: Update the ADR’s reconciliation decision, including the text around
the default-profile behavior, to explicitly document what happens when an absent
connection later returns: state that its prior Profile ownership is removed and
it is reassigned to the Default Profile, or document the retention rule that
prevents this. Include the resulting user-visible consequence and any relevant
privacy or security boundary impact.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: e1a716ad-cff0-4ae6-8f7e-d5adb7a4f034

📥 Commits

Reviewing files that changed from the base of the PR and between 20e6726 and 51bbb63.

📒 Files selected for processing (6)
  • .changeset/encrypted-mail-profile-foundation.md
  • CONTEXT.md
  • apps/unwired-mail/unwired-mail/Services/ProductSync/MailboxConnectionSyncModels.swift
  • apps/unwired-mail/unwired-mail/Services/ProductSync/MailboxConnectionSyncService.swift
  • apps/unwired-mail/unwired-mailTests/MailboxConnectionSyncServiceTests.swift
  • docs/adr/0048-separate-encrypted-mail-profile-ownership.md
📜 Review details
🧰 Additional context used
📓 Path-based instructions (7)
.changeset/**/*

📄 CodeRabbit inference engine (AGENTS.md)

Create a Changeset for runtime behavior changes or exported type/API changes; tests-only, internal refactors, documentation-only changes, and JSDoc-only maintenance may omit one at maintainer discretion.

Files:

  • .changeset/encrypted-mail-profile-foundation.md
docs/adr/**/*

📄 CodeRabbit inference engine (AGENTS.md)

Record durable architecture decisions and privacy-boundary decisions in docs/adr/.

Files:

  • docs/adr/0048-separate-encrypted-mail-profile-ownership.md
**/*.md

⚙️ CodeRabbit configuration file

**/*.md: Treat docs as product truth, not marketing. Prefer precise domain language
from CONTEXT.md; flag undefined terms, synonym drift, and conflicts with
existing ADRs under docs/adr/. For ADR changes, require a clear decision,
consequences, and privacy or security boundary impact; flag silent
reversals of prior ADRs without an explicit supersession note. For
docs/agents/ and bootstrap docs, verify agent workflow, validation, and
triage guidance stay aligned with AGENTS.md. Flag broken cross-links,
speculative future claims presented as shipped behavior, and docs that
omit the Apple vs TypeScript ownership of a boundary they describe.

Files:

  • docs/adr/0048-separate-encrypted-mail-profile-ownership.md
  • CONTEXT.md
CONTEXT.md

📄 CodeRabbit inference engine (AGENTS.md)

Update CONTEXT.md when product language, domain terms, or resolved ambiguity changes.

Files:

  • CONTEXT.md
apps/unwired-mail/**/*.{swift,m,mm}

📄 CodeRabbit inference engine (AGENTS.md)

The Apple app must provide formatter, linter, and test commands. Run zsh scripts/check-apple-lint.zsh for format and lint, and use the documented xcodebuild test command for tests; SwiftLint runs in strict mode.

Files:

  • apps/unwired-mail/unwired-mailTests/MailboxConnectionSyncServiceTests.swift
  • apps/unwired-mail/unwired-mail/Services/ProductSync/MailboxConnectionSyncModels.swift
  • apps/unwired-mail/unwired-mail/Services/ProductSync/MailboxConnectionSyncService.swift
apps/unwired-mail/**/*.swift

📄 CodeRabbit inference engine (AGENTS.md)

Apple validation must use SwiftLint strict mode and Swift Format strict linting; if Apple tooling is unavailable, report that clearly in the final handoff.

Files:

  • apps/unwired-mail/unwired-mailTests/MailboxConnectionSyncServiceTests.swift
  • apps/unwired-mail/unwired-mail/Services/ProductSync/MailboxConnectionSyncModels.swift
  • apps/unwired-mail/unwired-mail/Services/ProductSync/MailboxConnectionSyncService.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/MailboxConnectionSyncServiceTests.swift
  • apps/unwired-mail/unwired-mail/Services/ProductSync/MailboxConnectionSyncModels.swift
  • apps/unwired-mail/unwired-mail/Services/ProductSync/MailboxConnectionSyncService.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/MailboxConnectionSyncServiceTests.swift
🪛 markdownlint-cli2 (0.23.2)
.changeset/encrypted-mail-profile-foundation.md

[warning] 5-5: First line in a file should be a top-level heading

(MD041, first-line-heading, first-line-h1)

🔇 Additional comments (8)
.changeset/encrypted-mail-profile-foundation.md (1)

1-3: LGTM!

Also applies to: 5-5

apps/unwired-mail/unwired-mail/Services/ProductSync/MailboxConnectionSyncModels.swift (2)

1-1: LGTM!

Also applies to: 170-260


324-331: LGTM!

Also applies to: 334-350, 352-370

docs/adr/0048-separate-encrypted-mail-profile-ownership.md (1)

1-15: LGTM!

Also applies to: 19-26, 28-29

apps/unwired-mail/unwired-mail/Services/ProductSync/MailboxConnectionSyncService.swift (3)

75-75: LGTM!

Also applies to: 104-109


525-550: LGTM!


588-637: LGTM!

Also applies to: 639-667, 713-746, 748-759, 762-772

apps/unwired-mail/unwired-mailTests/MailboxConnectionSyncServiceTests.swift (1)

1089-1125: LGTM!

Also applies to: 1151-1175, 1177-1226, 1228-1250, 1416-1418

Comment thread CONTEXT.md
Comment thread docs/adr/0048-separate-encrypted-mail-profile-ownership.md Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Create the Mail Profile model and lossless Default Profile migration

1 participant