feat(connectivity): dual-strategy presence/inbox/invites + refcounted merge (#47 C1) - #71
Conversation
…(C1 phase 1) pair.ts was the only dual-strategy call site; inbox, presence, and invite send were Nostr-only. Post-v1.3.1 that was incoherent: a friend behind a Nostr-blocking firewall can now be ADDED (offline ContactCard) but then showed permanently offline, every invite died, and the friendship was inert. - mergeRooms gains refcounted per-peer transport tracking, registered once at construction and fanned out to subscribers: join fires when the FIRST transport sees a peer, leave only when the LAST loses them. (The old shared-dedup-set fired leave on a single-transport drop and starved second subscribers — tolerable for short-lived pairing rooms, wrong for long-lived ones.) - presence (own + friend rooms), the inbox, and the invite send path pass strategies ['nostr','mqtt']. Duplicate delivery over both transports is absorbed per consumer: idempotent heartbeat stamps, the PR-18 (from, nonce) inbox replay guard, the latched invite-ACK. - the invite timeout's network-down verdict now judges both transports (pairingRelaysUnreachable, same PR-21 rationale as pairing) - session rooms deliberately stay Nostr-only: two transport rooms would open duplicate RTCPeerConnections and double video — needs its own design pass. ARCHITECTURE §4.2 updated. Fixes #47 item C1 (phase 1 as scoped by the issue). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
📜 Recent review details⏰ Context from checks skipped due to timeout. (4)
🧰 Additional context used📓 Path-based instructions (7)**/*📄 CodeRabbit inference engine (CLAUDE.md)
Files:
src/**/*.{ts,tsx,css}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
src/**/*.{ts,tsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
src/**/*.{ts,tsx,rs}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.{ts,tsx,rs}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.{ts,tsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.test.ts📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🔇 Additional comments (7)
📝 WalkthroughWalkthroughDiscovery data rooms now explicitly race Nostr and MQTT. Merged rooms track peers across transports with refcounted join/leave events, while invite reachability and architecture documentation reflect the updated behavior. Media sessions remain Nostr-only. ChangesDual-transport discovery
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant FriendClient
participant DiscoveryRooms
participant Nostr
participant MQTT
participant InviteConsumer
FriendClient->>DiscoveryRooms: join invite, inbox, or presence room
DiscoveryRooms->>Nostr: establish data-room transport
DiscoveryRooms->>MQTT: establish data-room transport
Nostr-->>InviteConsumer: deliver envelope or heartbeat
MQTT-->>InviteConsumer: deliver envelope or heartbeat
InviteConsumer->>InviteConsumer: replay guard or idempotent handling
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
✨ Simplify code
Warning Tools execution failed with the following error: Failed to run tools: 13 INTERNAL: Received RST_STREAM with code 2 (Internal server error) Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
What
pair.tswas the only dual-strategy call site; inbox, presence, invite send, and session rooms were Nostr-only (ARCHITECTURE §4.2). Post-v1.3.1 this was incoherent: a friend behind a Nostr-blocking firewall can now be added (offline ContactCard) but then shows permanently offline, every invite dies, and no session can rendezvous — the friendship is inert.Changes
mergeRoomsrefcounted peer tracking (prerequisite found during this pass): the old shared-dedup-set fired the consumer'sonPeerLeavewhen ONE transport dropped even though the peer was still connected on the other, and its per-subscriber registration starved second subscribers. Tracking is now registered once at construction (mirroringwrapRoom's fan-out): join fires on the first transport that sees a peer, leave only when the last loses them.strategies: ['nostr', 'mqtt']. Duplicate delivery of one logical message over both transports is absorbed per consumer — idempotent heartbeat stamps, the PR-18(from, nonce)inbox replay guard, the latched invite-ACK (C2) — documented at the merge site.pairingRelaysUnreachable(PR-21 rationale) so a Nostr-blocked/MQTT-working network isn't misreported as down.Interplay with I48 noted per the issue: a persistent inbox/presence MQTT connection sidesteps the recorded reconnect-leak; the justification here is functionality.
Addresses item C1 of #47 (phase 1 as scoped).
Tests / gates
trystero-merge.test.ts+2: single-transport drop fires no leave / last-transport drop fires one; multi-subscriber fan-out.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation