Conversation
|
Claude finished @yewreeka's task —— View job Code Review: DMs on Single Inbox PlanThis is an excellent plan document that demonstrates strong architectural thinking and clear communication. The shift from the multi-inbox approach in #398 to this single-inbox design is a significant simplification enabled by ADR 011. ✅ StrengthsArchitectural Soundness
Clarity and Completeness
Code Quality Considerations
|
9b35b91 to
e600dd1
Compare
54945db to
91fd59f
Compare
Adds docs/plans/dm-single-inbox.md — v1 plan for private DMs rebuilt on the single-inbox identity model (PR #713 / ADR 011). Core shift from #398: - No `convos.org/convo_request` content type. Origin signal rides on `ConversationCustomMetadata.originGroupId` (immutable, sender-set at DM creation, spoof-protected by membership verification on receive). - No back-channel DM between group inboxes. Peer DMs are native XMTP DMs between stable single-inbox inboxIds. - No new custom consent primitive. Uses `XMTPiOS.PrivatePreferences` for per-conversation and per-inboxId consent, which streams across the user's devices for free. Requires extending `XMTPClientProvider` to expose `client.preferences` (Convos today only uses conversation-level consent). - `allows_dms: bool` public bit still rides on `ProfileUpdate` metadata, same mechanism as #398, unchanged. - Select-members allow-list still private via self-addressed XMTP DMs. Policy model: - App-level toggle = default for newly joined groups. - Per-group tri-state: off / everyone / select-members. - New members never auto-populate the select-list. Product decision baked in: no Requests bucket. Every DM resolves to `.allowed` or silent `.denied`; `.unknown` is transient only. Home-list query excludes both `.unknown` and `.denied` to avoid flicker. Six UAQs left to debate before leaving Draft; v2 deferrals called out (group spinoffs, explicit profile-choice sheet).
91fd59f to
9c065e8
Compare
Summary
Draft v1 plan for private DMs rebuilt on top of the single-inbox identity model from #713, replacing the multi-inbox-era design in #398.
The architecture shift: every user now has exactly one stable inboxId (ADR 011). That collapses "DMs" from a custom back-channel protocol into two stock XMTP calls —
conversations.newConversation(with:)andConsentState. No new content types, no new codecs, noallow_dmsprofile-metadata flag, no self-addressed config stream. Just UI + glue.Stacked on
single-inbox-refactor(PR #713) because the entire plan is predicated on that merging. If #713 bounces, this plan goes with it.What's in the plan
newConversation(with: inboxId)→ DM opens with origin-group profile inherited.unknown → .allowed / .denied), no separate request box.allowedwhen pair shares N ≥ 2 groups?.unknownDMsWhy this is ~150 lines instead of ~500
The previous spec in #398 had to invent the
convos.org/convo_requestcontent type, a back-channel DM between group-inboxIds, anallows_dmsmetadata flag on profile messages, a self-addressed XMTP message stream for the select-members list, and disappearing origin-context tied to message expiration — entirely because every user had many inboxes and there was no direct way to DM someone. Single-inbox dissolves all of that.Review asks
swift-architect.Test plan (for the plan)
newConversationprimitive actually does what's claimed here, and that the per-conversation profile inheritance fits the existingDBMemberProfilepipeline.🤖 Generated with Claude Code
Note
Add plan document for DMs on a single inbox
Adds a 1-pager design plan at docs/plans/dm-single-inbox.md covering the single-inbox model for direct messages. The document outlines sender/receiver flows using XMTP consent states, v1 non-goals, mockups, FAQs, and next steps. Supersedes #398.
📊 Macroscope summarized e600dd1. 62 files reviewed, 7 issues evaluated, 3 issues filtered, 0 comments posted
🗂️ Filtered Issues
ConvosCore/Sources/ConvosCore/Config/ConfigManager.swift — 0 comments posted, 2 evaluated, 1 filtered
gatewayUrloverride is computed at lines 115-117 but not passed toConvosConfigurationfor thedevenvironment at lines 142-149. This meansoverrides.gatewayURLis silently ignored for dev builds. Compare to thelocalenvironment (lines 126-134) which correctly passes bothxmtpEndpointandgatewayUrl. [ Posting failed ]ConvosCore/Sources/ConvosCore/Inboxes/SessionStateMachine.swift — 0 comments posted, 1 evaluated, 1 filtered
.stopaction is not handled for.authorizing,.registering, or.authenticatingBackendstates - it falls through to thedefaultcase which only logs a warning. If a caller invokesstop()while the state machine is mid-authorization or mid-registration, the stop request is silently ignored and the operation continues to completion, potentially transitioning all the way to.readystate despite the stop request. This defeats the caller's expectation thatstop()will halt the state machine. [ Posting failed ]ConvosCore/Sources/ConvosCore/Messaging/UnusedConversationCache.swift — 0 comments posted, 2 evaluated, 1 filtered
try await optimisticConversation.publish()completes but beforegroup = xmtpGroupassignment, the code catchesCancellationErrorand returns without callingleaveGroup(). The MLS group is now live on the XMTP network but orphaned locally with no reference to clean it up. The comment claims "the post-publish branch below owns rollback" but that branch is never reached whenCancellationErroris caught in the firstdoblock beforegroupis assigned. [ Posting failed ]