Skip to content

Add JoinRequest custom content type with optional profile and metadata#556

Merged
yewreeka merged 6 commits intodevfrom
jarod/join-request-content-type
Mar 6, 2026
Merged

Add JoinRequest custom content type with optional profile and metadata#556
yewreeka merged 6 commits intodevfrom
jarod/join-request-content-type

Conversation

@yewreeka
Copy link
Copy Markdown
Contributor

@yewreeka yewreeka commented Mar 5, 2026

  • New JoinRequestContent model with inviteSlug (required), profile
    (optional), and metadata map (optional) for extensible fields like
    deviceName and confirmationCode
  • JoinRequestCodec for encoding/decoding with JSON serialization
  • Update InviteCoordinator.sendJoinRequest to send structured content
    type instead of plain text slug
  • Update InviteCoordinator.processMessage to handle both new content
    type and legacy plain text for backward compatibility
  • Update processDm and lastMessageAsSignedInvite for both formats
  • Pass profile and metadata through JoinRequest, JoinResult, and
    JoinRequestResult
  • Register JoinRequestCodec in XMTP client codecs
  • 17 new tests for codec encode/decode, edge cases, and compatibility

Note

Add JoinRequest custom content type with optional profile and metadata to invite flows

  • Adds JoinRequestContent and JoinRequestCodec in JoinRequestCodec.swift as a new XMTP content type carrying an invite slug plus optional joiner profile (name, avatar, member kind) and arbitrary metadata.
  • InviteCoordinator.sendJoinRequest now sends a structured JoinRequestContent message first, then a plain text slug as a backward-compatible fallback.
  • InviteCoordinator.processMessage and processDm accept both the new content type and legacy plain text, propagating profile and metadata through JoinRequest and JoinResult.
  • InviteJoinRequestsManager persists the joiner's profile to the local database (DBMember/DBMemberProfile) when present; all call sites now pass a DatabaseWriter to the manager.
  • JoinRequestCodec is registered on XMTP clients created by InboxStateMachine, StreamProcessor, and SyncingManager.

Macroscope summarized 5227da4.

Copy link
Copy Markdown
Contributor Author

yewreeka commented Mar 5, 2026


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
Copy link
Copy Markdown

claude Bot commented Mar 5, 2026

Claude finished @yewreeka's task —— View job


PR Review Complete

I've thoroughly reviewed this PR. Here's my detailed feedback:


Overview

This PR successfully adds a custom XMTP content type (convos.org/join_request v1.0) that extends join requests with optional profile data and metadata. The implementation is well-structured with good backward compatibility, but there are several issues to address.


Critical Issues

1. Partial Send Failure Creates False Error State ⚠️ InviteCoordinator.swift:123-127

Macroscope correctly identified a critical issue. The sendJoinRequest method sends two messages sequentially:

// Line 123: Send structured content
_ = try await dm.send(content: joinRequest, options: .init(contentType: codec.contentType))
// Line 127: Send plain text fallback
_ = try await dm.send(content: slug)

Problem: If the second send throws after the first succeeds, the caller receives an exception even though the structured message was already delivered and the receiver can process it independently. This leads to:

  • False failure UI for the user
  • Potential duplicate retry attempts
  • Inconsistent state between sender and receiver

Recommendation:

  • Wrap both sends in a single transaction-like operation with error recovery
  • OR catch and log the second send failure without propagating it (since the first message is sufficient)
  • OR send only the structured content type (legacy clients are already unsupported by other recent changes)

Code Quality Issues

2. Missing Input Validation JoinRequestCodec.swift:60-65

The codec doesn't validate the inviteSlug field:

public func encode(content: JoinRequestContent) throws -> EncodedContent {
    var encodedContent = EncodedContent()
    encodedContent.type = ContentTypeJoinRequest
    encodedContent.content = try JSONEncoder().encode(content)
    return encodedContent
}

Issues:

  • Empty inviteSlug is accepted (would fail later during decryption)
  • No length limits on metadata keys/values (could cause issues with very large payloads)
  • imageURL in JoinRequestProfile isn't validated as a valid URL

Recommendation:

guard !content.inviteSlug.isEmpty else { 
    throw JoinRequestCodecError.emptySlug 
}
// Consider max length for metadata values

3. Inconsistent Error Handling InviteCoordinator.swift:354-368

The processDm method silently ignores malformed messages:

let candidates = messages.filter { message in
    guard let contentType = try? message.encodedContent.type,
          contentType == ContentTypeText || contentType == ContentTypeJoinRequest,
          message.senderInboxId != client.inviteInboxId else {
        return false
    }
    return true
}

Issue: Using try? swallows decoding errors without logging. This makes debugging difficult when messages fail to process.

Recommendation: Log decode failures at debug level for troubleshooting.

4. Potential Race Condition InviteJoinRequestsManager.swift:93-117

The persistJoinerProfile method is called asynchronously but isn't awaited in the caller:

// Line 56: Fire and forget
await persistJoinerProfile(result)

Issue: If profile persistence fails, there's no error propagation or retry mechanism. The member is added to the group but their profile might not be saved.

Recommendation: Either:

  • Make profile persistence synchronous (await + handle errors)
  • OR add logging/metrics for persistence failures
  • OR implement a retry mechanism

Architecture & Design

5. Good: Protocol-Based Abstraction

The PR correctly uses InviteClientProvider protocol wrappers instead of direct XMTP SDK types, following the project's testability patterns.

6. Good: Backward Compatibility

The dual-message approach (structured + plain text) ensures compatibility with legacy clients, though this creates the issue noted in #1.

7. Good: Profile Persistence IntegrationInviteJoinRequestsManager.swift:93-117

The profile data is automatically persisted to DBMemberProfile with support for the memberKind field (agent detection). This is well-integrated with the existing profile system.


Test Coverage

8. Excellent Unit Test CoverageJoinRequestCodecTests.swift

17 tests covering:

  • Encode/decode with all field combinations
  • Edge cases (empty metadata, special characters, long slugs)
  • Error cases (empty content, malformed JSON, missing slug)
  • Forward compatibility (unknown fields ignored)

9. Good Integration TestsInviteJoinRequestIntegrationTests.swift

Tests cover:

  • End-to-end join flow with new content type
  • Backward compatibility with plain text
  • Tag revocation
  • Profile and metadata propagation

Missing test coverage:

  • The partial send failure scenario (issue [Screen] Onboarding #1)
  • Metadata validation limits
  • Invalid URL in profile imageURL

Performance Considerations

10. Minor: Redundant Message Processing InviteCoordinator.swift:123-127

Sending both structured and plain text messages doubles network traffic. Consider:

  • Sending only structured content (with fallback in same message via XMTP's fallback mechanism)
  • OR deprecating plain text support entirely if all clients are now on newer versions

Security Review

11. No Security Issues Identified

  • SignedInvite cryptographic verification is preserved
  • No new credential exposure
  • Metadata is arbitrary but scoped per-request (no injection risks)
  • Profile data is validated before persistence

SwiftLint Compliance

12. Follows Project Conventions

Code adheres to CLAUDE.md guidelines:

  • No force unwrapping (!)
  • No implicitly unwrapped optionals
  • Proper use of guard for early returns
  • Appropriate use of @preconcurrency for XMTP types
  • No trailing whitespace or formatting issues
  • Proper modifier order

13. Codec RegistrationInboxStateMachine.swift:1077

The JoinRequestCodec is properly registered in the client options alongside other custom codecs.


Summary

Recommendation: Request Changes

This is a well-implemented feature with good test coverage and proper architectural patterns. However, the partial send failure issue (#1) is a critical bug that must be fixed before merging. The other issues are lower priority but should be addressed for production readiness.

Priority fixes:

  1. ⚠️ Critical: Fix partial send failure handling in sendJoinRequest (issue [Screen] Onboarding #1)
  2. Add input validation for inviteSlug and metadata (issue Contact Card Creation UI #2)
  3. Improve error logging for decode failures (issue Privy Integration #3)
  4. Handle profile persistence failures (issue Add PostHog for Analytics #4)

Nice to have:

  • Add test coverage for partial send failure scenario
  • Consider deprecating dual-message approach for simplicity

Files Changed: 12 files (+668/-479)
Test Coverage: 17 new unit tests + 5 integration tests
| Branch

Comment thread ConvosInvites/Sources/ConvosInvites/InviteCoordinator.swift
@yewreeka yewreeka marked this pull request as ready for review March 6, 2026 03:34
@macroscopeapp
Copy link
Copy Markdown
Contributor

macroscopeapp Bot commented Mar 6, 2026

Approvability

Verdict: Needs human review

This adds a new custom content type and database persistence for join request profiles, introducing user‑facing behavior and writes. I’d recommend a human verify the new functionality and data changes.

You can customize Macroscope's approvability policy. Learn more.

yewreeka added 4 commits March 5, 2026 22:00
- New JoinRequestContent model with inviteSlug (required), profile
  (optional), and metadata map (optional) for extensible fields like
  deviceName and confirmationCode
- JoinRequestCodec for encoding/decoding with JSON serialization
- Update InviteCoordinator.sendJoinRequest to send structured content
  type instead of plain text slug
- Update InviteCoordinator.processMessage to handle both new content
  type and legacy plain text for backward compatibility
- Update processDm and lastMessageAsSignedInvite for both formats
- Pass profile and metadata through JoinRequest, JoinResult, and
  JoinRequestResult
- Register JoinRequestCodec in XMTP client codecs
- 17 new tests for codec encode/decode, edge cases, and compatibility
- Add JoinRequestCodec and InviteJoinErrorCodec to test client codecs
- Add integration test: JoinRequestContent type message adds member
- Add integration test: InviteCoordinator.sendJoinRequest sends
  structured content type with profile and metadata fields
- Existing plain text tests preserved for backward compatibility
Old clients without JoinRequestCodec registered will fall back to
TextCodec for unknown content types, but TextCodec decodes the raw
JSON bytes rather than the fallback string, causing
SignedInvite.fromURLSafeSlug to fail on the JSON. Sending a second
plain text message ensures old clients can still process join requests.

The receiver processes whichever message it encounters first. The
duplicate is harmless since processDm returns after the first success
and addMembers with an existing member is a no-op failure.
@yewreeka yewreeka force-pushed the jarod/join-request-content-type branch from 16b87f5 to 3acd5b7 Compare March 6, 2026 06:00
@yewreeka yewreeka changed the base branch from jarod/convos-vault-plan to graphite-base/556 March 6, 2026 06:01
@yewreeka yewreeka force-pushed the graphite-base/556 branch from e7a7b29 to 10343f3 Compare March 6, 2026 06:01
@yewreeka yewreeka changed the base branch from graphite-base/556 to dev March 6, 2026 06:01
When a join request includes a profile (name, imageURL, memberKind),
write it to DBMemberProfile so the new member's display name and
avatar appear immediately without waiting for a ProfileUpdate message.

Maps memberKind "agent" to DBMemberKind.agent for agent identification.
@yewreeka yewreeka merged commit 240dbc0 into dev Mar 6, 2026
8 of 9 checks passed
@yewreeka yewreeka deleted the jarod/join-request-content-type branch March 6, 2026 06:39
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