Skip to content

Verify Network backend local loopback - #31

Closed
rchatham wants to merge 2 commits into
feature/network-migration-backend-selectorfrom
feature/network-migration-local-loopback
Closed

Verify Network backend local loopback#31
rchatham wants to merge 2 commits into
feature/network-migration-backend-selectorfrom
feature/network-migration-local-loopback

Conversation

@rchatham

@rchatham rchatham commented Aug 6, 2026

Copy link
Copy Markdown
Owner

Summary

Adds the first end-to-end Network backend loopback verification and fixes the issues found while debugging discovery/connectivity on simulator/local Network.framework runs.

Changes

  • Use NWBrowser.Descriptor.bonjourWithTXTRecord so browser results include the discovery TXT metadata needed to map endpoints to PeerIdentity.
  • Use short UUID-backed local identities for Network-backed managers instead of archived MCPeerID identifiers, avoiding TXT-record truncation and self-discovery confusion.
  • Avoid dispatch reentrancy crashes by emitting Network coordinator observer events outside the coordinator serial queue.
  • Switch experimental Network parameters to TCP for now; public docs now explicitly warn that .networkFramework is unencrypted/unauthenticated and not for sensitive data until hardening lands.
  • Add NetworkPeerLoopbackTests that verifies two Network-backed managers can:
    • discover each other
    • connect automatically
    • exchange a typed PeerMessage
    • stop cleanly
  • Add demo launch arguments for simulator E2E debugging:
    • PCNetworkBackend
    • PCAutoStart
    • PCDisplayName <name>

Debug findings

  • Discovery was occurring at the Network.framework level, but browser results had metadata: <none> because the browser descriptor was not requesting TXT records.
  • Connections failed with NWError internal TLS errors when using bare TLS options without an identity/trust configuration.
  • Once TCP connected, observer delivery could crash with dispatch_sync called on queue already owned by current thread; coordinator events now leave the serial queue before notifying observers.

Verification

  • swift test — 95 tests passed
  • xcodebuild test -project PeerConnectivity.xcodeproj -scheme PeerConnectivity -destination 'platform=iOS Simulator,name=iPhone 16,OS=18.3.1' -configuration Debug — 95 tests passed
  • swift test --filter NetworkPeerLoopbackTests after doc updates — passed
  • Built PeerConnectivityDemo for iOS Simulator
  • Installed/launched demo on two booted simulators with Network backend auto-start arguments for Alice/Bob; Network logs showed Bonjour resolution and TCP connections between the two simulator apps
  • Correctness reviewer — no merge-blocking findings
  • Security reviewer — no merge-blocking security findings after public plaintext/unauthenticated warnings were added

Limitations / follow-ups

  • Network backend remains explicit opt-in and experimental.
  • Secure transport hardening is still required: app-configurable TLS identity, PSK, or another authenticated trust model.
  • Stream/resource transfer and SwiftUI browser replacement are still follow-up work.

Stack context

Stacked on PR #29 (feature/network-migration-backend-selector). This is the local loopback verification slice of the Network migration stack.

@rchatham
rchatham force-pushed the feature/network-migration-local-loopback branch from e260287 to 83edcbb Compare August 6, 2026 21:11
…d-selector' into HEAD

# Conflicts:
#	PeerConnectivity.xcodeproj/project.pbxproj
#	PeerConnectivityDemo/ViewController.swift
#	Sources/NetworkPeerCoordinator.swift
#	Sources/PeerConnectionManager.swift

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds an initial end-to-end loopback verification path for the experimental Network.framework backend, while addressing discovery/identity mapping and a coordinator observer reentrancy crash encountered during simulator/local Network.framework debugging.

Changes:

  • Update Network browsing to request Bonjour TXT records and adjust Network-backed peer identity to avoid TXT truncation/self-discovery confusion.
  • Prevent coordinator queue reentrancy crashes by emitting observer events outside the coordinator’s serial queue; switch experimental Network parameters to TCP.
  • Add Network backend loopback XCTest coverage and document/demo runtime switches (including explicit plaintext/unauthenticated warnings).

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
Sources/PeerConnectionManager.swift Adds explicit warnings about Network backend security properties and uses a Network-specific peer identity initializer when backend is .networkFramework.
Sources/Peer.swift Introduces a Network-specific local peer initializer using a UUID-backed PeerIdentity identifier.
Sources/NetworkPeerTransport.swift Switches Network transport parameters to TCP and uses bonjourWithTXTRecord for discovery metadata.
Sources/NetworkPeerCoordinator.swift Avoids observer delivery while holding the coordinator serial queue by emitting events after the synchronized state update.
README.md Documents the experimental Network backend and clearly warns about current lack of encryption/authentication.
PeerConnectivityTests/NetworkPeerLoopbackTests.swift Adds a first e2e loopback test for Network-backed managers (discovery, connect, typed message exchange, stop).
PeerConnectivityDemo/ViewController.swift Adds launch-argument support for Network backend selection, auto-start, and explicit display name for simulator debugging.
PeerConnectivity.xcodeproj/project.pbxproj Registers the new loopback test file in the Xcode test target.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread PeerConnectivityTests/NetworkPeerLoopbackTests.swift
Comment thread PeerConnectivityTests/NetworkPeerLoopbackTests.swift
Comment thread PeerConnectivityDemo/ViewController.swift
@rchatham

Copy link
Copy Markdown
Owner Author

Superseded by consolidated PR #42. No commits were discarded; this branch is preserved as a recovery and reference point. Addressed review findings were replied to and resolved before consolidation.

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.

2 participants