refactor: decompose transport layer into composable modules

[1amKhush]

What Changed

This PR extracts CEP-specific routing and capability logic from the monolithic transport files into focused, composable modules — zero logic changes, zero public API changes, zero test modifications.

Extractions

New Module	Extracted From	Responsibility
nostr-server/inbound-notification-dispatcher.ts	authorizeAndProcessEvent	CEP-41 open-stream frame routing (abort, ping, accept) + CEP-22 oversized frame processing
nostr-server/outbound-response-router.ts	handleResponse	Response routing, CEP-22 oversized send, CEP-41 pending response deferral, pricing tag injection
nostr-server/open-stream-factory.ts	handleIncomingRequest + flushPendingOpenStreamResponse	OpenStreamWriter lifecycle and pending response flush
nostr-client/inbound-notification-dispatcher.ts	handleNotification	Client-side CEP-41/CEP-22 frame interception
capability-negotiator.ts	Both transports + discovery-tags.ts	ServerCapabilityNegotiator and ClientCapabilityNegotiator — tag building, gift-wrap kind selection, discovery state

Architecture After

nostr-server-transport.ts (1199 lines, was 1561)
├── InboundNotificationDispatcher   — CEP-22/41 frame interception
├── OutboundResponseRouter          — response routing + oversized send
├── ServerOpenStreamFactory         — writer lifecycle
├── ServerCapabilityNegotiator      — tag building + gift-wrap selection
└── core auth + middleware dispatch (unchanged)

nostr-client-transport.ts (1170 lines, was 1285)
├── ClientInboundNotificationDispatcher — CEP-22/41 frame interception
├── ClientCapabilityNegotiator          — tag building + gift-wrap selection
└── core send + discovery (unchanged)

Design Decisions

• Dependency injection via typed deps bags — each extracted class takes a Deps interface in its constructor, keeping it decoupled from the transport class hierarchy. No inheritance, no abstract base classes.
• Maps shared by reference — the openStreamWriters and pendingOpenStreamResponses maps are owned by ServerOpenStreamFactory and shared with the outbound router via getWritersMap() / getPendingResponsesMap(). This avoids duplicating state while keeping ownership clear.
• discovery-tags.ts deleted — its standalone functions (hasSingleTag, learnPeerCapabilities, etc.) moved into capability-negotiator.ts alongside the new negotiator classes. No separate file needed.
• ClientCapabilityNegotiator state encapsulation — server capability flags are updated via learnServerCapabilities() and setServerInitializeEvent() methods rather than exposed as public mutable fields.

Verification

• bun run build — clean, no type errors
• bun test src/transport/ — 254 tests pass, 0 failures, zero test files modified

[Copilot]

Pull request overview

Refactors the Nostr transport layer by extracting CEP-22/CEP-41 routing, open-stream lifecycle, and capability negotiation logic from the monolithic client/server transport implementations into smaller, dependency-injected modules (intended to preserve behavior and public API).

Changes:

• Introduces server-side modules for inbound notification interception, outbound response routing, and open-stream writer lifecycle management.
• Introduces client-side inbound notification interception and a shared capability negotiation module (replacing discovery-tags.ts).
• Updates nostr-server-transport.ts and nostr-client-transport.ts to delegate responsibilities to the extracted modules.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
src/transport/nostr-server/outbound-response-router.ts	New server response router handling announcement responses, open-stream deferral, oversized transfer, and pricing tag injection.
src/transport/nostr-server/open-stream-factory.ts	New factory owning open-stream writers and pending final responses, with flush-on-close/abort behavior.
src/transport/nostr-server/inbound-notification-dispatcher.ts	New server interceptor for CEP-41 open-stream frames and CEP-22 oversized frames (including accept handling and synthetic dispatch).
src/transport/nostr-server-transport.ts	Wires the new server modules and removes inlined CEP-22/41/capability logic from the transport class.
src/transport/nostr-client/inbound-notification-dispatcher.ts	New client interceptor for CEP-41/CEP-22 progress frames, routing synthetic completions appropriately.
src/transport/nostr-client-transport.ts	Wires the new client modules and delegates capability negotiation and frame interception logic.
src/transport/discovery-tags.ts	Deleted; its functionality moved into the shared capability negotiator module.
src/transport/capability-negotiator.ts	New shared capability negotiation + discovery-tag parsing utilities for both client and server transports.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[ContextVM-org]

Solid refactor that improves structure. My only recommendation is to treat it as an intermediate step, since NostrServerTransport and NostrClientTransport are still quite large, and the shared mutable maps exposed by ServerOpenStreamFactory.getWritersMap() and ServerOpenStreamFactory.getPendingResponsesMap() could be encapsulated more tightly in a later pass. Maybe we can also find other opportunities to reduce the size of the transports. Also keep JSDoc consistency, and follow agents.md

[ContextVM-org]

I think this is moving in the right direction and the latest extractions are meaningful. Modules like ServerEventPipeline, ClientEventPipeline, ServerOpenStreamFactory, ClientOpenStreamFactory, and OutboundResponseRouter improve the structure without feeling artificial.

I would still push this further before considering the refactor complete. The main remaining issue is that NostrServerTransport.authorizeAndProcessEvent() and NostrClientTransport.processIncomingEvent() are still carrying too much orchestration logic. To me, the best next step is not more small helper extraction, but pulling those inbound flows into dedicated coordinators that own parsing, capability learning, classification, routing, and middleware dispatch while keeping the transport classes as the public façade.

More broadly, I think the guiding principle here should be to extract along real workflow and protocol boundaries, not just to reduce line count. When a module owns a full concern like event unwrapping, open-stream lifecycle, response routing, or outbound request sending, the code gets healthier because responsibilities become clearer and easier to reason about. When extraction is only cosmetic, the transports get fragmented without really becoming simpler.

With that in mind, I would also look at NostrClientTransport.sendRequest() as the next client-side extraction point, since it still mixes normal send, CEP-22 branching, tag construction, and correlation registration. I would use the same lens elsewhere too: keep the transports as façade entrypoints, and keep moving protocol-specific state transitions and branching into focused collaborators whenever they form a coherent subflow.

My recommendation is to keep pushing in that direction. I see the refactor as healthy, and I think this PR is a good opportunity to add more meaningful modularization so the transports end up slimmer internally while preserving the same external API.

Would love to hear your thoughts on this.

[1amKhush]

@ContextVM-org Thanks for the thoughtful feedback and architectural direction! I completely agree with your assessment. When I looked at extracting authorizeAndProcessEvent earlier, I hesitated because it felt like it would require passing too much internal state around, which risked feeling like "artificial" extraction just for the sake of line counts.

Framing it as pulling these flows into dedicated coordinators makes perfect sense. Treating the main transport classes as clean, public façades while delegating the heavy protocol orchestration (parsing, capability learning, branching, and routing) to cohesive sub-modules is definitely the right move for the long-term health of the codebase.

Based on your feedback, I'll proceed with the following extractions:

1. ServerInboundCoordinator: To take ownership of authorizeAndProcessEvent(), handling capability learning, authorization gating, and middleware dispatch.
2. ClientInboundCoordinator: To take ownership of processIncomingEvent(), managing the server capability discovery upgrades and routing responses.
3. ClientOutboundSender: To encapsulate sendRequest(), handling the CEP-22 branching, tag construction, and correlation registration.

Do you think this direction makes sense? Lmk your thoughts on this, then ill get to the refactor!

[ContextVM-org]

I think this direction makes sense and is the right way to take the PR from a good cleanup to a strong final architecture. I agree that the goal should not be to extract code just to reduce line count, but to make NostrServerTransport and NostrClientTransport true public façades: they keep the external API, lifecycle, and top-level send/receive entrypoints, while the protocol workflows live in focused internal modules.

The three proposed extractions feel meaningful to me. ServerInboundCoordinator is a good boundary because the current server inbound flow is doing a full workflow: parsing, authorization, capability learning, correlation registration, request decoration, notification interception, and middleware dispatch. ClientInboundCoordinator also makes sense because client inbound processing currently mixes discovery learning, initialize-event upgrades, response classification, envelope tracking, and dispatch. ClientOutboundSender is also a strong candidate because outbound sending already has a clear decision boundary around normal sends vs CEP-22, tag construction, gift-wrap choice, and correlation registration.

The main thing I would watch is dependency shape. If these coordinators need to receive half of the transport internals, the abstraction will start feeling artificial. I would try to pass narrow capabilities and existing collaborators instead: stores, negotiators, factories, dispatchers, logger/error hooks, and small callbacks like send message, forward message, or create tags. The coordinator should own the workflow, but it should not become a mirror of the transport class.

More broadly, I would use this as the design principle for the final shape of the PR: extract around protocol and lifecycle boundaries, not around arbitrary chunks of code. A good module should answer a clear question, like “how do I unwrap and validate this event?”, “how do I dispatch an inbound server message?”, “how do I route an outbound response?”, “how do I send this client request?”, or “what do we know about peer capabilities?”. If a future feature naturally fits one of those questions, it should have an obvious place to land without cluttering the transports again.

So yes, I would proceed with those three extractions. After that, I would do one more pass and check whether client-side server discovery/metadata deserves a small internal state object, since initialize events, capability-bearing list envelopes, and support flags are a coherent concern. I would also check whether server outbound notification routing/broadcasting still feels dense enough to deserve its own router. If those areas are clean after the coordinator work, I would stop there rather than over-splitting.

My ideal final bar for this PR is: transports remain stable public façades, inbound and outbound workflows are owned by named coordinators/senders/routers, stores own state, protocol modules own protocol mechanics, dependency interfaces stay narrow and explicit, and the code follows the existing conventions and JSDoc expectations. That would give us a maintainable architecture now and a much better foundation for future transport features.

[1amKhush]

Acknowledged

[ContextVM-org]

PR looks good, I would just refine AGENTS.md a bit as a polishing step. I think the current refactor still matches the document well, but this PR made two things clearer that are worth stating explicitly in the architecture section: the ~700 LOC target is a heuristic, and the real goal is clear ownership boundaries; and modularization should happen around protocol or lifecycle concerns, not arbitrary code splitting. I would keep it to a very small wording update, because the overall guidance already feels right.

[1amKhush]

PR looks good, I would just refine AGENTS.md a bit as a polishing step. I think the current refactor still matches the document well, but this PR made two things clearer that are worth stating explicitly in the architecture section

Great call. Agreed that this is worth codifying since the PR itself proved the principle. Up for review, thanks!