Skip to content

Fix pending invite stuck after navigation#514

Merged
graphite-app[bot] merged 1 commit into
devfrom
fix/pending-invite-stuck-after-navigation
Feb 24, 2026
Merged

Fix pending invite stuck after navigation#514
graphite-app[bot] merged 1 commit into
devfrom
fix/pending-invite-stuck-after-navigation

Conversation

@yewreeka

@yewreeka yewreeka commented Feb 24, 2026

Copy link
Copy Markdown
Contributor

Problem

When a joiner scans an invite and navigates away before the inviter approves, the conversation permanently stays as a "pending invite" and never transitions to an active conversation. This was reported by Andy Yang — stuck for 45+ minutes on "Balloon Animals".

Root Cause

Three gaps in the sync/stream lifecycle:

  1. No post-sync group discovery — after syncing, the app never checked for XMTP groups that were added while streams were stopped
  2. Resume didn't re-synchandleResume() only restarted streams without re-syncing, missing groups added during pause
  3. ValueObservation cancelled on navigation — the DB observation watching for the draft→active transition was tied to the conversation view's lifecycle

Fix

1. Post-sync group discovery (SyncingManager)

New discoverNewConversations() method lists all XMTP groups after every sync and processes any missing from the local DB. Acts as a safety net for missed stream events.

2. Re-sync on resume (SyncingManager)

handleResume() now calls syncAllConversations + discoverNewConversations + processJoinRequestsAfterSync instead of only restarting streams.

3. Restart observation on navigation (ConversationStateMachine)

handleUseExisting() checks if the conversation is still a draft with an inviteTag and restarts waitForJoinedConversation observation in the background.

4. Accessibility fix

Added accessibilityIdentifier("close-new-conversation") to the close button on NewConversationView.

Testing

  • 59 unit tests pass
  • QA test 24 (pending invite recovery) passed end-to-end:
    • Joiner scans invite → navigates away → inviter approves via CLI → conversation automatically transitions to active → messaging works

Files Changed

  • ConvosCore/Sources/ConvosCore/Syncing/SyncingManager.swift — post-sync discovery + resume re-sync
  • ConvosCore/Sources/ConvosCore/Inboxes/ConversationStateMachine.swift — restart pending invite observation
  • Convos/Conversation Creation/NewConversationView.swift — close button accessibility ID
  • docs/plans/fix-pending-invite-stuck.md — plan doc
  • qa/tests/24-pending-invite-recovery.md — QA test

Note

Recover pending invites after navigation by observing draft inviteTag in ConversationStateMachine.handleUseExisting and backfilling missed conversations in SyncingManager

Add background observation for draft invites in ConversationStateMachine to transition to the joined conversation, and backfill missed conversations on sync and resume in SyncingManager; also add a close button accessibility identifier in the new conversation view. Key entry points: ConvosCore/Sources/ConvosCore/Inboxes/ConversationStateMachine.swift, ConvosCore/Sources/ConvosCore/Syncing/SyncingManager.swift, Convos/Conversation Creation/NewConversationView.swift.

📍Where to Start

Start with ConversationStateMachine.handleUseExisting and the new helpers in ConversationStateMachine.swift, then review SyncingManager.handleSyncComplete and handleResume plus discoverNewConversations in SyncingManager.swift.

Macroscope summarized d422b19.

@yewreeka yewreeka force-pushed the fix/pending-invite-stuck-after-navigation branch from 3f67e9c to 580d8e7 Compare February 24, 2026 22:44
@claude

claude Bot commented Feb 24, 2026

Copy link
Copy Markdown

Claude finished @yewreeka's task —— View job


Code Review — Fix Pending Invite Stuck After Navigation

I've reviewed the changes and overall this is a solid fix for the stuck invite problem. The three-pronged approach (post-sync discovery, resume re-sync, and observation restart) effectively addresses the root causes. However, there are some potential issues and improvements worth considering.


Code Quality & Best Practices

✅ Strong Points

  • Clear separation of concerns between SyncingManager (discovery) and ConversationStateMachine (observation)
  • Good use of GRDB ValueObservation for reactive state transitions
  • Comprehensive logging that will help debug future sync issues
  • Well-documented plan matching the implementation

⚠️ Areas for Improvement

1. State validation in handlePendingInviteResolved (ConversationStateMachine.swift:492-506)

The guard checks if the state is still .ready with matching draft ID, but there's a potential edge case:

guard case .ready(let result) = _state,
      result.conversationId == draftConversationId else {
    return
}

Issue: If a user rapidly switches between draft conversations (scan invite A → navigate away → scan invite B → A resolves), the check only validates against the current draft ID but doesn't verify that the observation belongs to the current draft.

Suggestion: Consider tracking the draft conversation ID that initiated each observation to prevent stale observations from overwriting the current state.


Potential Bugs

1. Cached message writer not invalidated (ConversationStateMachine.swift:500)

When transitioning from draft to real conversation, cachedMessageWriter is cleared, which is correct. However, in startPendingInviteObservationIfNeeded, if an observation is already running, it's cancelled but no state cleanup happens.

Edge case: User scans invite A (observation starts) → navigates away → scans invite B (calls useExisting with new draft) → A resolves first → cachedMessageWriter points to A but state shows B.

Resolution: The current code at line 500 (cachedMessageWriter = nil) does handle this, so this is actually fine. The Macroscope review comment about this was addressed.

2. Discovery loop efficiency (SyncingManager.swift:420-456)

The discoverNewConversations method lists all groups and checks existence for each:

for group in groups where !existingIds.contains(group.id) {
    try await streamProcessor.processConversation(group, params: params)
}

Potential issue: If a user has many groups (100+), this runs on every sync completion and resume. The current implementation fetches all conversation IDs into a Set, which is good, but processing happens serially.

Suggestion: Consider:

  • Limiting discovery to groups created in the last N hours on resume (reduces unnecessary processing)
  • Adding a timestamp check to skip discovery if sync was recent (< 30s ago)
  • Parallel processing with controlled concurrency (TaskGroup with maxConcurrentTasks)

Trade-off: Current approach is safe and correct; optimization is only needed if performance issues arise.


Performance Considerations

1. Re-sync on resume (SyncingManager.swift:594-604)

Resume now calls syncAllConversations, which fetches all group messages from the network. This is necessary but could be slow on large accounts.

Observation: The PR description mentions "groups added while paused" — this suggests limiting sync scope to recent changes would be acceptable.

Suggestion: Consider using syncAllConversations(consentStates:lastSyncedAt:) if the SDK supports incremental sync, or track the last sync timestamp to limit discovery to recent groups.

Current status: Acceptable as-is since syncAllConversations is already used elsewhere. Monitor performance in production.

2. Database query in hot path (ConversationStateMachine.swift:461-466)

startPendingInviteObservationIfNeeded performs a synchronous database read:

let inviteTag: String? = try? databaseReader.read { db in
    try DBConversation
        .filter(DBConversation.Columns.id == draftConversationId)
        .select(DBConversation.Columns.inviteTag)
        .fetchOne(db)
}

Status: This is fine — selecting a single column with indexed ID is fast. GRDB handles this efficiently.


Security Concerns

✅ No security issues identified. The changes:

  • Don't expose sensitive data
  • Use existing authentication patterns
  • Follow the codebase's consent state filtering (consentStates: [.allowed, .unknown])

Test Coverage

✅ Covered

  • Plan document exists with clear problem statement
  • QA test provides end-to-end validation
  • PR description mentions "59 unit tests pass"

⚠️ Gaps

The PR doesn't include new unit tests for:

  1. SyncingManager.discoverNewConversations

    • Should test: Groups missing from DB are processed after sync
    • Should test: Existing groups are not re-processed
    • Should test: Errors in processing one group don't block others
  2. ConversationStateMachine.startPendingInviteObservationIfNeeded

    • Should test: Observation starts for draft with inviteTag
    • Should test: Observation doesn't start for draft without inviteTag
    • Should test: Observation doesn't start for non-draft conversation
    • Should test: State transitions correctly when observation fires
  3. Edge cases

    • Multiple rapid draft switches
    • Observation fires after state machine is stopped/reset

Recommendation: Add unit tests to prevent regressions. The QA test is good for validation but not sufficient for CI.


Architecture & Patterns

✅ The fix follows existing patterns:

  • State machine transitions via action queue
  • Actor isolation for thread safety
  • GRDB ValueObservation for reactive queries

⚠️ Minor concern: observationTask is now cancelled in multiple places (stop, delete, deinit, pending resolution). Consider extracting a cancelObservation() helper to avoid duplication.


Accessibility

✅ The accessibility fix (line 56 in NewConversationView.swift) is unrelated to the main fix but is a good addition:

.accessibilityIdentifier("close-new-conversation")

This enables UI testing for the close button.


Summary & Recommendations

Must Fix (P0)

  • None — the implementation is functional as-is

Should Fix (P1)

  1. Add unit tests for discoverNewConversations and startPendingInviteObservationIfNeeded
  2. Consider adding a timestamp check to skip discovery if last sync was recent (< 30s)

Nice to Have (P2)

  1. Extract cancelObservation() helper to reduce duplication
  2. Monitor syncAllConversations performance on resume in production
  3. Consider incremental sync if SDK supports it

Overall Assessment

Approve with minor suggestions. The fix is well-designed and addresses all three gaps identified in the root cause analysis. The lack of unit tests is the main concern, but the QA test provides good coverage for the happy path. The implementation is safe to merge.


@yewreeka yewreeka force-pushed the fix/pending-invite-stuck-after-navigation branch from 580d8e7 to 19353bc Compare February 24, 2026 22:49
@yewreeka yewreeka force-pushed the fix/pending-invite-stuck-after-navigation branch 2 times, most recently from 34d77c2 to e6abb51 Compare February 24, 2026 22:57

yewreeka commented Feb 24, 2026

Copy link
Copy Markdown
Contributor Author

Merge activity

## Problem

When a joiner scans an invite and navigates away before the inviter approves, the conversation permanently stays as a "pending invite" and never transitions to an active conversation. This was reported by Andy Yang — stuck for 45+ minutes on "Balloon Animals".

## Root Cause

Three gaps in the sync/stream lifecycle:

1. **No post-sync group discovery** — after syncing, the app never checked for XMTP groups that were added while streams were stopped
2. **Resume didn't re-sync** — `handleResume()` only restarted streams without re-syncing, missing groups added during pause
3. **ValueObservation cancelled on navigation** — the DB observation watching for the draft→active transition was tied to the conversation view's lifecycle

## Fix

### 1. Post-sync group discovery (`SyncingManager`)
New `discoverNewConversations()` method lists all XMTP groups after every sync and processes any missing from the local DB. Acts as a safety net for missed stream events.

### 2. Re-sync on resume (`SyncingManager`)
`handleResume()` now calls `syncAllConversations` + `discoverNewConversations` + `processJoinRequestsAfterSync` instead of only restarting streams.

### 3. Restart observation on navigation (`ConversationStateMachine`)
`handleUseExisting()` checks if the conversation is still a draft with an inviteTag and restarts `waitForJoinedConversation` observation in the background.

### 4. Accessibility fix
Added `accessibilityIdentifier("close-new-conversation")` to the close button on `NewConversationView`.

## Testing

- 59 unit tests pass
- QA test 24 (pending invite recovery) passed end-to-end:
  - Joiner scans invite → navigates away → inviter approves via CLI → conversation automatically transitions to active → messaging works

## Files Changed

- `ConvosCore/Sources/ConvosCore/Syncing/SyncingManager.swift` — post-sync discovery + resume re-sync
- `ConvosCore/Sources/ConvosCore/Inboxes/ConversationStateMachine.swift` — restart pending invite observation
- `Convos/Conversation Creation/NewConversationView.swift` — close button accessibility ID
- `docs/plans/fix-pending-invite-stuck.md` — plan doc
- `qa/tests/24-pending-invite-recovery.md` — QA test

<!-- Macroscope's pull request summary starts here -->
<!-- Macroscope will only edit the content between these invisible markers, and the markers themselves will not be visible in the GitHub rendered markdown. -->
<!-- If you delete either of the start / end markers from your PR's description, Macroscope will append its summary at the bottom of the description. -->
> [!NOTE]
> ### Recover pending invites after navigation by observing draft `inviteTag` in `ConversationStateMachine.handleUseExisting` and backfilling missed conversations in `SyncingManager`
> Add background observation for draft invites in `ConversationStateMachine` to transition to the joined conversation, and backfill missed conversations on sync and resume in `SyncingManager`; also add a close button accessibility identifier in the new conversation view. Key entry points: [`ConvosCore/Sources/ConvosCore/Inboxes/ConversationStateMachine.swift`](https://github.com/xmtplabs/convos-ios/pull/514/files#diff-a1762ce564035d6cdd7e3b2d3200cfe0db6c3ec5fac03636392780086ba58344), [`ConvosCore/Sources/ConvosCore/Syncing/SyncingManager.swift`](https://github.com/xmtplabs/convos-ios/pull/514/files#diff-ba2362cf646b78283a1af17654ea8d2c18f6b1cb686a78e7761e10f1b0328101), [`Convos/Conversation Creation/NewConversationView.swift`](https://github.com/xmtplabs/convos-ios/pull/514/files#diff-be8eeec29bfb3f245f16cd6258d1ef28ced1aa75bad52187e34243f915b084f6).
>
> #### 📍Where to Start
> Start with `ConversationStateMachine.handleUseExisting` and the new helpers in [ConversationStateMachine.swift](https://github.com/xmtplabs/convos-ios/pull/514/files#diff-a1762ce564035d6cdd7e3b2d3200cfe0db6c3ec5fac03636392780086ba58344), then review `SyncingManager.handleSyncComplete` and `handleResume` plus `discoverNewConversations` in [SyncingManager.swift](https://github.com/xmtplabs/convos-ios/pull/514/files#diff-ba2362cf646b78283a1af17654ea8d2c18f6b1cb686a78e7761e10f1b0328101).
>
> <!-- Macroscope's review summary starts here -->
>
> <sup><a href="https://app.macroscope.com">Macroscope</a> summarized e6abb51.</sup>
> <!-- Macroscope's review summary ends here -->
>
<!-- macroscope-ui-refresh -->
<!-- Macroscope's pull request summary ends here -->
@graphite-app graphite-app Bot force-pushed the fix/pending-invite-stuck-after-navigation branch from e6abb51 to d422b19 Compare February 24, 2026 23:00
@graphite-app graphite-app Bot merged commit d422b19 into dev Feb 24, 2026
8 checks passed
@graphite-app graphite-app Bot deleted the fix/pending-invite-stuck-after-navigation branch February 24, 2026 23:02
Comment on lines 755 to 756

// Cancel observation tasks and stop accepting new messages
// Note: currentTask is already cancelled by delete() - don't cancel ourselves!
messageStreamContinuation?.finish()

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.

🟡 Medium Inboxes/ConversationStateMachine.swift:755

Cleanup doesn’t fully reset message‑sending state. Consider clearing cachedMessageWriter during teardown (e.g., in handleDelete()/cleanUpState()) and resetting isMessageStreamSetup synchronously so a quick restart doesn’t skip stream setup or reuse a stale writer.

 await clearInviteJoinErrorHandler()
 
+        cachedMessageWriter = nil
         messageStreamContinuation?.finish()
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file ConvosCore/Sources/ConvosCore/Inboxes/ConversationStateMachine.swift around lines 755-756:

Cleanup doesn’t fully reset message‑sending state. Consider clearing `cachedMessageWriter` during teardown (e.g., in `handleDelete()`/`cleanUpState()`) and resetting `isMessageStreamSetup` synchronously so a quick restart doesn’t skip stream setup or reuse a stale writer.

Evidence trail:
ConvosCore/Sources/ConvosCore/Inboxes/ConversationStateMachine.swift lines 93, 198-202, 205-221, 744-774, 861-868 at REVIEWED_COMMIT. `cachedMessageWriter` is declared at line 93, set at 219, but never cleared in `handleDelete()` or `cleanUpState()`. `resetMessageStream()` (lines 198-202) resets `isMessageStreamSetup` but is only called asynchronously via `onTermination` (line 182) or task completion (line 194), not synchronously in cleanup functions.

yewreeka added a commit that referenced this pull request Feb 25, 2026
PR #514 added syncAllConversations on resume to pick up changes
while paused/backgrounded, but the test still asserted it should
not be called. Update to match current behavior.
yewreeka added a commit that referenced this pull request Feb 25, 2026
PR #514 added syncAllConversations on resume to pick up changes
while paused/backgrounded, but the test still asserted it should
not be called. Update to match current behavior.
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.

1 participant