Refactor: extract Discovery interface from SymNode - #4
Merged
Conversation
SymNode was 929 lines with TCP server + Bonjour discovery + business logic tightly coupled. Tests couldn't exercise lifecycle without spawning child processes and opening network sockets. Refactoring: - Extract Discovery base class (EventEmitter interface) - BonjourDiscovery: TCP server + dns-sd/bonjour-service (existing behavior) - NullDiscovery: no-op for relay-only nodes and tests - SymNode accepts opts.discovery for injection (defaults based on relayOnly) - SymNode: 929 → 762 lines. Discovery: 258 lines (new file) Testability improvements: - Node tests inject NullDiscovery — no network, no child processes - Discovery tests validate TCP server, handshake protocol, rejection - 77 tests total (10 new), all passing on macOS and Linux CI Design principle: separate what changes (networking) from what doesn't (memory, coupling, encoding). Each can now evolve independently. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
bonjour-service's multicast UDP socket leaks on destroy() (upstream issue), preventing clean process exit on Linux CI. Add mdns: false option for server-only mode — tests validate TCP handshake protocol without mDNS. Default is mdns: true (full Bonjour behavior unchanged for production). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Extract networking (TCP server + Bonjour discovery) from SymNode into a pluggable Discovery interface. SymNode drops from 929 to 762 lines. Networking is now separately testable.
Before: SymNode owned TCP server, dns-sd processes, bonjour-service, and peer handshakes inline. Tests couldn't exercise lifecycle without real networking.
After: Three Discovery implementations:
BonjourDiscovery— existing LAN behavior (TCP + dns-sd/bonjour-service)NullDiscovery— no-op for relay-only nodes and testsDiscovery— base class (EventEmitter interface)SymNode accepts
opts.discoveryfor injection. Defaults: BonjourDiscovery (normal) or NullDiscovery (relayOnly).MMP Spec Compliance
Section 5 — Connection (Layer 2): Discovery behavior unchanged. The interface boundary matches the spec's separation between transport (L1) and connection (L2).
Testing
77 tests total (10 new), all passing:
discovery.test.js: NullDiscovery, BonjourDiscovery TCP server, handshake validation, non-handshake rejectionnode.test.js: updated to inject NullDiscovery — validates business logic without networkingChecklist
npm test) — 77 pass, 0 failopts.discoveryis optional, defaults preserved)