Skip to content

Merge main hotfixes into dev for 1.1.0 release#522

Merged
yewreeka merged 9 commits into
devfrom
merge/main-hotfixes-to-dev
Feb 25, 2026
Merged

Merge main hotfixes into dev for 1.1.0 release#522
yewreeka merged 9 commits into
devfrom
merge/main-hotfixes-to-dev

Conversation

@yewreeka

@yewreeka yewreeka commented Feb 25, 2026

Copy link
Copy Markdown
Contributor

Incorporates hotfixes from main branch before promoting release 1.1.0.

Changes from main hotfixes:

  • Removed 'anonymous' terminology from UI copy
  • Raised FD soft limit to 512

Kept from dev (unchanged):

  • 24-hour stalePendingInviteInterval (hotfix had 7 days)
  • Log.debug levels for pending invite operations (hotfix upgraded to Log.info)
  • Asset renewal manager feature
  • Pending invites in debug view (not settings)
  • Full deletion logic for stale pending invites

This PR must be merged before running make promote-release to enable fast-forward merge from dev to main.

Note

Merge main hotfixes into dev for 1.1.0 and adjust ConvosCoreTests.DeleteExpiredPendingInvitesTests.testSkipsRecentInvites to seed createdAt as 2 hours ago

Update the test to use a 2-hour threshold for recent invites and add redacted QA SQLite assets in qa/cxdb.

📍Where to Start

Start with the test change in DeleteExpiredPendingInvitesTests.swift.

Macroscope summarized a74424e.

onefovrth and others added 5 commits February 10, 2026 17:45
- Update Quickname detail copy from 'You always start anonymous' to 'You control how you show up'
- Simplify join event message to always say 'You joined as [Name]' instead of 'You joined anonymously as [Name]'
The default iOS soft limit of 256 file descriptors is insufficient for
users with ~50 conversations, as each XMTP conversation requires
database connections and sockets. Raises the soft limit to 512 at app
launch before any XMTP clients are created.

Also bumps build number to 3 for hotfix resubmission.
The 1.0.6 train is closed on App Store Connect. Bumps marketing version
to 1.0.7 and resets build number to 1 for the hotfix submission.
- Add maxAwakePendingInvites (default 3) to limit how many pending
  invite XMTP clients are active simultaneously. Previously, all
  pending invite inboxes were force-woken on app launch regardless
  of the maxAwakeInboxes limit, which could result in 50+ active
  clients for users who scanned many invite codes without approval.

- Detect stale pending invites older than 7 days on app launch.
  Automatic deletion is temporarily disabled — stale invites are
  logged but not removed, pending validation via the debug view.

- Only consider invites with <= 1 conversation member as deletable.
  Invites where the user has actually been added to the group (more
  than 1 member) are never deleted, preventing accidental data loss.
  Additionally, inboxes with any other conversations are preserved.

- Update sleep() to allow sleeping over-cap pending invite inboxes
  while still protecting inboxes under the cap.

- Update attemptWake() to enforce the pending invite cap when both
  the pending cap and overall capacity are exceeded.

- Add PendingInviteDebugView accessible from Settings (temporarily
  visible in production) showing all pending invites with creation
  date, wait duration, member count, and client/inbox IDs. Expired
  invites (> 7 days) are highlighted in a separate section with a
  hold-to-delete button that fully cleans up the inbox record,
  keychain identity, and lifecycle manager tracking.

- Inbox and keychain deletion failures are caught individually so
  one failure does not prevent cleanup of remaining invites.

- Add PendingInviteDetail model (with memberCount) and repository
  methods for the debug view and stale invite queries.

- Add 17 new tests: 6 for pending invite cap behavior, 4 for stale
  expiry detection, and 7 for deleteExpiredPendingInvites covering
  basic deletion, multi-member protection, other-conversations
  protection, recent invite preservation, lifecycle manager cleanup,
  partial failure resilience, and empty state.
@claude

claude Bot commented Feb 25, 2026

Copy link
Copy Markdown

Claude finished @yewreeka's task —— View job


Code Review Complete ✅

Tasks

  • Read changed test file (DeleteExpiredPendingInvitesTests.swift)
  • Review code quality and best practices
  • Check for potential bugs or issues
  • Evaluate performance considerations
  • Assess security concerns
  • Verify test coverage
  • Provide comprehensive feedback

Summary

This PR successfully merges main hotfixes into dev. The two post-merge commits (7ed6d32 and 53f7708) address issues flagged by the Macroscope bot. Overall, the changes are well-executed with good attention to detail.

Detailed Review

Code Quality & Best Practices

Positive observations:

  • Clean resolution of merge conflicts with clear documentation in the PR description
  • Test file properly updated to reflect the 2-hour recency baseline (changed from 1 hour/2 days in conflict)
  • Good use of descriptive variable names (twoHoursAgo is clearer than oneHourAgo)
  • Proper adherence to Swift testing conventions with the Testing framework

String interpolation fix (53f7708):

// Before (broken):
Log.debug("Skipped \\((clientIdsToKeep.count\)) inbox(es)...")

// After (fixed):
Log.debug("Skipped \(clientIdsToKeep.count) inbox(es)...")

This correctly fixes the escaped backslash issue that would have printed literal text instead of the count value.

Bug Fixes

Duplicate function call removal (7ed6d32):
The duplicate await cleanupStalePendingInvites() call at InboxLifecycleManager.swift:456 was correctly removed. This would have caused:

  • Unnecessary performance overhead on app launch
  • Duplicate logging output
  • Redundant database queries

Both issues were properly identified by Macroscope and correctly resolved.

Test Coverage

The testSkipsRecentInvites() test properly validates the 24-hour stale pending invite interval:

  • Uses twoHoursAgo for test data (well within the 24-hour window)
  • Expects deleted == 0 (no deletion should occur)
  • Verifies inbox preservation with inboxCount == 1

Test suite comprehensiveness:
The test file covers all critical scenarios:

  1. ✅ Basic deletion of expired invites
  2. ✅ Multi-member protection (don't delete active conversations)
  3. ✅ Other conversations protection
  4. ✅ Recent invites protection (the changed test)
  5. ✅ Lifecycle manager cleanup
  6. ✅ Partial failure resilience
  7. ✅ Empty state handling

Performance Considerations

  • Removing the duplicate cleanupStalePendingInvites() call improves app launch performance
  • The cleanup operation involves database queries and can be expensive with many pending invites
  • The 24-hour interval (kept from dev) is a reasonable balance between cleanup frequency and performance

Security Concerns

SQLite files (qa/cxdb/qa.sqlite*):
These are QA test database assets. Security notes:

  • ✅ Located in qa/ directory (test assets, not production code)
  • ✅ PR description states they are "redacted QA SQLite assets"
  • ⚠️ Recommendation: Verify these files don't contain any real user data or sensitive keys before merging
  • ⚠️ Consider adding .gitattributes rule to prevent diff viewing: qa/**/*.sqlite* binary

Logging considerations:

  • The fixed log message at line 614 uses Log.debug, appropriate for operational logging
  • No sensitive data exposed in log messages

💡 Suggestions (Optional)

  1. Test naming consistency: Consider renaming the time variable to match the actual interval being tested:

    // Current: tests 2 hours against 24-hour window
    let twoHoursAgo = Date().addingTimeInterval(-2 * 60 * 60)
    
    // Alternative (more explicit about what's being tested):
    let withinTwentyFourHours = Date().addingTimeInterval(-2 * 60 * 60)

    Not critical - the current naming is clear enough.

  2. Magic number extraction: The 2-hour test value could be a constant:

    private enum Constant {
        static let testRecentInviteAge: TimeInterval = 2 * 60 * 60
    }

    But this is minor since it's only used once.

  3. Binary file storage: The SQLite files add ~12KB to the repo. If these grow over time, consider:

    • Using Git LFS for binary test assets
    • Generating test databases programmatically instead of checking them in

🎯 Merge Recommendation

✅ APPROVED - This PR is ready to merge after addressing the security verification:

Before merging:

  • Confirm qa/cxdb/qa.sqlite* files contain no real user data or API keys
  • Consider adding git attributes for binary files

Strengths:

  • Previous issues (duplicate call, broken interpolation) resolved correctly
  • Test coverage is comprehensive and properly updated
  • Merge conflict resolution is well-documented
  • Code follows project conventions (no force unwrapping, proper logging, async/await patterns)

Risk level: Low - changes are minimal post-merge fixes with good test coverage.


Comment thread ConvosCore/Sources/ConvosCore/Inboxes/InboxLifecycleManager.swift
Incorporates hotfixes from main:
- Removed 'anonymous' terminology from UI copy
- Raised FD soft limit to 512

Kept dev's original values:
- 24-hour stalePendingInviteInterval (not 7 days from hotfix)
- Log.debug levels for pending invite operations (not Log.info from hotfix)
- Asset renewal manager
- Pending invites in debug view (not settings)
- Full deletion logic for stale pending invites
@yewreeka yewreeka force-pushed the merge/main-hotfixes-to-dev branch from 14e25db to 671fe5f Compare February 25, 2026 05:13
Comment thread ConvosCore/Sources/ConvosCore/Inboxes/InboxLifecycleManager.swift
@yewreeka yewreeka merged commit 4677e43 into dev Feb 25, 2026
4 of 5 checks passed
@yewreeka yewreeka deleted the merge/main-hotfixes-to-dev branch February 25, 2026 05:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants