Fix wrong conversation showing after QR scan

[yewreeka]

Skip inviteTag-based conversation matching when the tag is empty.
When a discovered XMTP group has an empty inviteTag (metadata not yet
synced or read failure), the query filter(inviteTag == "") would match
any existing conversation that also has an empty tag, causing the wrong
conversation's clientConversationId to be merged with the new group.

Note

Gate merge-by-inviteTag in ConvosCore.Storage.Writers.ConversationWriter.store to run only when dbConversation.inviteTag is non-empty to fix wrong conversation showing after QR scan

Add a precondition in ConversationWriter.store so the inviteTag lookup and merge path runs only when dbConversation.inviteTag.isEmpty is false in ConversationWriter.swift.

📍Where to Start

Start with the store method in ConversationWriter.swift.

^{Macroscope summarized c2702b3.}

[yewreeka]

• Fix wrong conversation showing after QR scan #520 👈 (View in Graphite)
• dev

---

How to use the Graphite Merge Queue

Add the label merge-queue to this PR to add it to the merge queue.

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

[claude]

Claude finished @yewreeka's task —— View job

---

Code Review

• Read and analyze the code change
• Review for bugs and edge cases
• Evaluate security and performance
• Check test coverage needs
• Provide comprehensive feedback

---

Summary

This is a well-targeted fix that addresses a critical bug where empty inviteTag values were incorrectly matching existing conversations. The change is minimal, safe, and follows defensive programming principles.

Code Change Analysis

Location: ConversationWriter.swift:404-408

Change:

// Before
if let localConversation = try DBConversation
    .filter(DBConversation.Columns.inviteTag == dbConversation.inviteTag)
    .filter(DBConversation.Columns.clientConversationId != dbConversation.clientConversationId)
    .fetchOne(db) {

// After  
if !dbConversation.inviteTag.isEmpty,
   let localConversation = try DBConversation
    .filter(DBConversation.Columns.inviteTag == dbConversation.inviteTag)
    .filter(DBConversation.Columns.clientConversationId != dbConversation.clientConversationId)
    .fetchOne(db) {

✅ Strengths

1. Correct Problem Diagnosis: The empty string check prevents false positive matches when XMTP group metadata hasn't synced yet
2. Minimal Change: Only adds one guard condition without altering existing logic
3. Defensive Programming: Prevents accidental merging of unrelated conversations
4. Performance: Early exit avoids unnecessary database query when inviteTag is empty

🔍 Potential Issues & Considerations

1. Test Coverage Gap ⚠️

Severity: Medium

The existing tests in ClientConversationIdPriorityTests.swift don't cover the empty inviteTag scenario. Consider adding a test like:

@Test("Empty inviteTag does not match other conversations with empty inviteTag")
func testEmptyInviteTagDoesNotMatch() async throws {
    // Create two separate groups that both have empty inviteTag
    // Verify they don't get merged/matched
}

This would prevent regression and document the expected behavior.

2. Edge Case: Whitespace-Only Tags

Severity: Low

The fix checks for empty string ("") but not whitespace-only strings (" "). While unlikely with XMTP's protocol, consider whether .trimmingCharacters(in: .whitespaces).isEmpty would be more robust.

Current: !dbConversation.inviteTag.isEmpty
Alternative: !dbConversation.inviteTag.trimmingCharacters(in: .whitespaces).isEmpty

However, given that inviteTag comes from XMTP's SDK (try conversation.inviteTag), the current check is probably sufficient unless the protocol explicitly allows whitespace.

3. Database Schema Question

Severity: Info

Is inviteTag nullable in the database schema? If so, you might also need to handle nil cases. From the code, it appears to be non-optional String, so empty string is the only "no value" case.

🔒 Security

No concerns. This change reduces the attack surface by preventing unintended conversation merging.

⚡ Performance

Positive impact. Skips database query when inviteTag is empty, avoiding unnecessary filtering and fetchOne() call.

📝 Documentation

The PR description clearly explains the issue. Consider adding a code comment explaining why the empty check is necessary:

// Skip inviteTag matching if tag is empty to prevent false positives when
// XMTP group metadata hasn't synced yet (would match any existing conversation
// that also has an empty tag)
if !dbConversation.inviteTag.isEmpty,
   let localConversation = try DBConversation

Recommendation

✅ Approve with suggestions

The fix is correct and safe. Consider:

1. Adding a test case for the empty inviteTag scenario
2. Adding a brief inline comment explaining the rationale
3. Verifying whether whitespace-only tags are possible (low priority)

---