Add inline reply and mark-as-read notification actions

[yewreeka]

Changes

Adds two notification actions to message push notifications:

1. Inline Reply

Users can reply directly from the notification banner or lock screen via a UNTextInputNotificationAction. The reply is sent through the existing messaging pipeline — wakes the correct inbox, gets a message writer, and sends.

2. Mark as Read

Clears all delivered notifications for the conversation from the notification center.

Implementation

• NotificationAction constants in ConvosCore define identifiers shared between the main app and NSE
• NSE sets categoryIdentifier on all decoded message notifications
• App delegate registers the category at launch and routes action responses
• SessionManager.sendNotificationReply handles the inbox wake → message write → send flow

Files changed

• ConvosCore/Notifications/NotificationAction.swift (new)
• ConvosCore/Sessions/SessionManager.swift
• ConvosCore/Sessions/SessionManagerProtocol.swift
• ConvosCore/Inboxes/MockInboxesService.swift
• Convos/ConvosAppDelegate.swift
• NotificationService/NotificationService.swift

Note

Add inline reply and mark-as-read actions to push notifications

• Registers a UNNotificationCategory with Reply and Mark as Read actions on launch via a new registerNotificationCategories() helper in ConvosAppDelegate.swift.
• Inline replies extract the typed text from UNTextInputNotificationResponse and send it via SessionManager.sendNotificationReply(text:conversationId:), which wakes the inbox and obtains a message writer before sending.
• Mark-as-read clears delivered notifications for the thread; normal taps retain existing behavior via the new handleNotificationTap helper.
• Notification content now includes sender avatar data (fetched and decrypted from DBMemberProfile) and is backed by an INSendMessageIntent, enabling system-level messaging UI in the notification.
• Behavioral Change: all push notifications now carry a categoryIdentifier and an INSendMessageIntent donation, which may affect how iOS groups or displays them.

^{Macroscope summarized 6f17c52.}

[claude]

Claude finished @yewreeka's task —— View job

---

Code Review Complete ✅

Todo List:

• Read all changed files
• Review code quality and project conventions
• Check for potential bugs and error handling
• Assess security concerns
• Evaluate performance considerations
• Check test coverage
• Provide comprehensive feedback

---

Summary

This PR adds inline reply and mark-as-read notification actions to push notifications. The implementation is solid overall, with good architecture and proper error handling. Below are my detailed findings.

---

✅ Code Quality & Best Practices

Strengths

1. Clean architecture - Proper separation between ConvosCore (shared identifiers), NSE (notification processing), and main app (action handling)
2. Protocol-driven design - SessionManagerProtocol extension for notification reply follows the project's dependency injection pattern
3. Consistent naming - Uses project conventions (NotificationAction as constants enum, not NotificationActions)
4. SwiftFormat compliance - K&R braces, trailing closures, proper spacing throughout
5. Good error handling - Comprehensive error logging without exposing sensitive data

Minor Style Issues

❗ ConvosAppDelegate.swift:156-157 - Empty reply text is handled with guard, but could benefit from user feedback logging:

guard !replyText.isEmpty else { return }

Consider adding:

guard !replyText.isEmpty else { 
    Log.debug("Ignoring empty notification reply")
    return 
}

---

🐛 Potential Bugs & Issues

1. Race Condition in Inbox Wake + Send Flow

SessionManager.swift:544-575 - The sendNotificationReply method wakes the inbox and then waits up to 2 seconds for ready state with a busy-wait loop:

await wakeInboxForNotification(clientId: conversation.clientId, inboxId: conversation.inboxId)
let service = try await messagingService(for: conversation.clientId, inboxId: conversation.inboxId)
await service.inboxStateManager.ensureForeground()
for _ in 0..<20 {
    if case .ready = service.inboxStateManager.currentState { break }
    try await Task.sleep(for: .milliseconds(100))
}

Concerns:

• No error if ready state isn't reached after 2 seconds - the message send proceeds anyway
• Busy-wait pattern is inefficient (though only 100ms intervals)
• If the inbox doesn't become ready in time, writer.send(text:) might fail with a cryptic error

Recommendation:

// After the loop, verify ready state
guard case .ready = service.inboxStateManager.currentState else {
    Log.error("Inbox not ready after 2s wait for notification reply")
    throw SessionManagerError.inboxNotReady
}

2. Silent Failure on Conversation Not Found

SessionManager.swift:549-552 - When conversation is not found, the method returns silently instead of throwing:

guard let conversation = ... else {
    Log.error("Cannot reply: conversation \(conversationId) not found")
    return  // Silent failure
}

This was already flagged by Macroscope and marked as "no longer relevant" - let me verify if this was actually fixed:

Looking at the latest commit (418007a), the code still returns silently. However, this might be intentional since:

1. The app delegate calls this with try await but doesn't show user-facing errors
2. Logging the error is sufficient for debugging
3. From the user's perspective, the notification just doesn't respond (which is better than crashing)

Verdict: This is acceptable defensive programming, but the PR description should note this behavior.

3. Missing MockInboxesService Implementation Details

MockInboxesService.swift:33 - Mock returns empty implementation:

public func sendNotificationReply(text: String, conversationId: String) async throws {}

For unit tests, consider tracking calls:

public var repliesSent: [(text: String, conversationId: String)] = []
public func sendNotificationReply(text: String, conversationId: String) async throws {
    repliesSent.append((text, conversationId))
}

---

🔒 Security Concerns

✅ No Security Issues Found

1. Input validation - Empty text is filtered before sending (ConvosAppDelegate.swift:157)
2. No sensitive data logging - Error messages log conversation IDs only (which are safe)
3. Proper authentication - Reply flow uses existing message writer which handles authentication
4. No XSS/injection risks - Text input is passed directly to XMTP message writer without processing

---

⚡ Performance Considerations

1. Notification Action Registration on Every Launch

ConvosAppDelegate.swift:18 - registerNotificationCategories() is called on every app launch:

func application(_ application: UIApplication, didFinishLaunchingWithOptions ...) -> Bool {
    SentryConfiguration.configure()
    UNUserNotificationCenter.current().delegate = self
    registerNotificationCategories()  // Every launch

Impact: Minimal - UNNotificationCenter.setNotificationCategories() is fast and idempotent.

Verdict: ✅ Acceptable pattern (standard iOS practice).

2. Avatar Loading in NSE

NotificationService.swift:173 & MessagingService+PushNotifications.swift:390-423 - Avatar data is loaded synchronously during notification delivery:

let avatarData = await loadSenderAvatarData(
    senderInboxId: decodedMessage.senderInboxId,
    conversationId: conversationId
)

The loadSenderAvatarData method:

• Reads from GRDB (fast)
• Decrypts image data with EncryptedImageLoader.loadAndDecrypt (potentially slow for large images)
• Runs on background thread (NSE context)

Concerns:

• NSE has a ~30 second execution limit
• Image decryption could take several seconds for large avatars
• If decryption fails or times out, notification still displays (which is good)

Verdict: ✅ Acceptable - failure is handled gracefully, and avatars enhance UX significantly.

---

🧪 Test Coverage

Missing Tests

No unit tests found for:

1. Notification action handling - handleNotificationReply in ConvosAppDelegate
2. Mark-as-read functionality - clearDeliveredNotifications
3. sendNotificationReply - The new SessionManager method

Recommended Tests

SessionManagerTests.swift already has infrastructure for testing. Add:

@Test("sendNotificationReply sends message after waking inbox")
func testSendNotificationReply() async throws {
    let fixtures = try await makeIntegrationTestFixtures()
    
    // Create inbox and conversation
    let (service, _) = await fixtures.sessionManager.addInbox()
    let result = try await service.inboxStateManager.waitForInboxReadyResult()
    let conversationId = "test-conversation"
    
    // Store conversation in database
    try await fixtures.databaseManager.dbWriter.write { db in
        try DBConversation(...).insert(db)
    }
    
    // Send notification reply
    try await fixtures.sessionManager.sendNotificationReply(
        text: "Reply text",
        conversationId: conversationId
    )
    
    // Verify message was sent (check database or mock message writer)
}

Suggested test files:

• ConvosCore/Tests/ConvosCoreTests/NotificationActionsTests.swift - Test sendNotificationReply with various states
• ConvosTests/NotificationActionHandlerTests.swift - Test ConvosAppDelegate action routing

---

📋 SwiftLint Compliance

Checked against CLAUDE.md rules:

✅ No force unwrapping - Uses safe unwrapping throughout
✅ No implicitly unwrapped optionals
✅ No assertions - Uses Log.error instead
✅ Explicit types on properties - private var contentHandler: ((UNNotificationContent) -> Void)?
✅ Guard preference - Properly uses guard for early returns
✅ Trailing closures - Only for single-parameter closures
✅ No step counts in comments
✅ No all-caps emphasis - Comments use normal case

Recommendation: Run /lint before merging to verify.

---

🎯 Recommendations

High Priority

1. Add error handling for inbox not ready - Throw error if inbox isn't ready after timeout (SessionManager.swift:564-567)
2. Add unit tests - Cover sendNotificationReply, handleNotificationReply, and clearDeliveredNotifications

Medium Priority

3. Document silent failure behavior - Add comment explaining why conversation-not-found returns silently (SessionManager.swift:549-552)
4. Enhance mock for testing - Track sendNotificationReply calls in MockInboxesService

Low Priority

5. Add debug logging for empty replies - Log when empty text is ignored (ConvosAppDelegate.swift:157)

---

✨ Overall Assessment

This is a well-implemented feature that follows project conventions and integrates cleanly with the existing notification infrastructure. The architecture is sound, error handling is comprehensive, and security is properly maintained.

Verdict: ✅ Approve with minor recommendations

The main gap is test coverage for the new functionality. Consider adding tests before merging, especially for the critical sendNotificationReply flow.

---

[macroscopeapp]

Approvability

Verdict: Needs human review

This PR introduces new user-facing notification functionality (inline reply and mark-as-read actions) with new message-sending behavior triggered from notifications. As a new feature adding user-facing capabilities and runtime behavior changes, it warrants human review.

^{You can customize Macroscope's approvability policy. Learn more.}