-
Notifications
You must be signed in to change notification settings - Fork 1
refactor: add native transport seam under IRC client #3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
louzt
wants to merge
2
commits into
obbyworld:main
Choose a base branch
from
louzt:feat/native-transport-seam
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,84 @@ | ||
| # tobby Architecture | ||
|
|
||
| ## System overview | ||
|
|
||
| tobby is a terminal IRC client with a local-first architecture: | ||
|
|
||
| - React/OpenTUI render layer for the terminal UI | ||
| - Zustand slices for UI, messages, servers, and IRC state | ||
| - a custom IRC client wrapper on top of the ObsidianIRC IRC core | ||
| - a transport seam that currently defaults to raw TCP/TLS, but can be replaced natively | ||
|
|
||
| The important design decision for transport evolution is this: | ||
|
|
||
| > the IRC layer should not need to know whether bytes came from plain TCP/TLS, a QUIC session, or a future identity-based native overlay. | ||
|
|
||
| ## Current layering | ||
|
|
||
| ```mermaid | ||
| graph TD | ||
| UI[React / OpenTUI UI] | ||
| Store[Zustand Store Slices] | ||
| IRCSlice[ircSlice event wiring] | ||
| Client[tobby IRCClient wrapper] | ||
| TransportFactory[SocketTransportFactory seam] | ||
| TcpSocket[NodeTCPSocket net/tls] | ||
| Network[IRC server] | ||
|
|
||
| UI --> Store | ||
| Store --> IRCSlice | ||
| IRCSlice --> Client | ||
| Client --> TransportFactory | ||
| TransportFactory --> TcpSocket | ||
| TcpSocket --> Network | ||
| ``` | ||
|
|
||
| ## Why the transport seam matters | ||
|
|
||
| Historically, tobby created `NodeTCPSocket` directly inside `IRCClient.connect()`. That made the transport path effectively hardcoded. | ||
|
|
||
| Now the client asks a socket transport factory for an `ISocket` implementation. | ||
|
|
||
| That gives three benefits: | ||
|
|
||
| 1. future native transport work can be integrated without rewriting the IRC client | ||
| 2. tests can inject a fake transport cleanly | ||
| 3. a downstream fork can experiment with a QUIC/identity overlay without making the upstream IRC logic depend on an external engine repository | ||
|
|
||
| ## Native transport direction | ||
|
|
||
| The intended native evolution is not “replace IRC semantics”. It is: | ||
|
|
||
| - keep IRC framing and event handling where it is | ||
| - replace only the byte transport under the socket interface | ||
| - preserve a compatibility fallback to plain TCP/TLS | ||
|
|
||
| ```mermaid | ||
| graph LR | ||
| Client[tobby IRCClient] | ||
| Factory[SocketTransportFactory] | ||
| Native[Native Overlay Transport] | ||
| Fallback[NodeTCPSocket TCP/TLS] | ||
| Peer[Remote endpoint] | ||
|
|
||
| Client --> Factory | ||
| Factory --> Native | ||
| Factory --> Fallback | ||
| Native --> Peer | ||
| Fallback --> Peer | ||
| ``` | ||
|
|
||
| ## QUIC-oriented target shape | ||
|
|
||
| A future native transport can expose the same `ISocket` shape while internally using: | ||
|
|
||
| - QUIC connection setup | ||
| - unreliable datagrams for latency-sensitive traffic when appropriate | ||
| - bidirectional streams for reliable control data | ||
| - identity-bound session authorization before opening the data path | ||
|
|
||
| That should remain an internal implementation detail of the transport adapter, not something leaked into the store or UI layers. | ||
|
|
||
| ## Integration rule | ||
|
|
||
| If a future transport cannot satisfy the existing `ISocket` contract cleanly, it should be wrapped in an adapter rather than forcing the IRC/UI layers to learn transport-specific behavior. |
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| import { NodeTCPSocket } from './nodeTcpSocket' | ||
| import type { ISocket, SocketTransportContext, SocketTransportFactory } from './socketTypes' | ||
|
|
||
| const defaultSocketTransportFactory: SocketTransportFactory = ({ url }) => new NodeTCPSocket(url) | ||
|
|
||
| let socketTransportFactory: SocketTransportFactory = defaultSocketTransportFactory | ||
|
|
||
| export function createSocketTransport(url: string): ISocket { | ||
| return socketTransportFactory({ url }) | ||
| } | ||
|
|
||
| export function setSocketTransportFactory(factory: SocketTransportFactory): void { | ||
| socketTransportFactory = factory | ||
| } | ||
|
|
||
| export function resetSocketTransportFactory(): void { | ||
| socketTransportFactory = defaultSocketTransportFactory | ||
| } | ||
|
|
||
| export type { ISocket, SocketTransportContext, SocketTransportFactory } |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| export interface ISocket { | ||
| onopen: (() => void) | null | ||
| onmessage: ((event: { data: string }) => void) | null | ||
| onerror: ((error: Error) => void) | null | ||
| onclose: (() => void) | null | ||
|
|
||
| send(data: string): void | ||
| close(): void | ||
| readyState: number | ||
| } | ||
|
|
||
| export interface SocketTransportContext { | ||
| url: string | ||
| } | ||
|
|
||
| export type SocketTransportFactory = (context: SocketTransportContext) => ISocket |
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,56 @@ | ||
| import { afterEach, describe, expect, test, vi } from 'vitest' | ||
| import net from 'net' | ||
| import { NodeTCPSocket } from '../../src/lib/nodeTcpSocket' | ||
| import { | ||
| createSocketTransport, | ||
| resetSocketTransportFactory, | ||
| setSocketTransportFactory, | ||
| } from '../../src/lib/socketTransport' | ||
|
|
||
| describe('socketTransport', () => { | ||
| afterEach(() => { | ||
| resetSocketTransportFactory() | ||
| }) | ||
|
|
||
| test('allows a custom socket transport to be injected', () => { | ||
| const fakeSocket = { | ||
| onopen: null, | ||
| onmessage: null, | ||
| onerror: null, | ||
| onclose: null, | ||
| send: vi.fn(), | ||
| close: vi.fn(), | ||
| readyState: 1, | ||
| } | ||
|
|
||
| const factory = vi.fn(() => fakeSocket) | ||
| setSocketTransportFactory(factory) | ||
|
|
||
| const socket = createSocketTransport('irc://example.com:6667') | ||
|
|
||
| expect(factory).toHaveBeenCalledWith({ url: 'irc://example.com:6667' }) | ||
| expect(socket).toBe(fakeSocket) | ||
| }) | ||
|
|
||
| test('reset restores the default TCP/TLS transport', () => { | ||
| const sentinel = vi.fn(() => { | ||
| throw new Error('custom transport should have been reset') | ||
| }) | ||
| const fakeSocket = { | ||
| on: vi.fn().mockReturnThis(), | ||
| end: vi.fn(), | ||
| write: vi.fn(), | ||
| } as unknown as net.Socket | ||
|
|
||
| const connectSpy = vi.spyOn(net, 'connect').mockReturnValue(fakeSocket) | ||
|
|
||
| setSocketTransportFactory(sentinel) | ||
| resetSocketTransportFactory() | ||
|
|
||
| const socket = createSocketTransport('irc://127.0.0.1:65535') | ||
|
|
||
| expect(sentinel).not.toHaveBeenCalled() | ||
| expect(connectSpy).toHaveBeenCalledWith({ host: '127.0.0.1', port: 65535 }) | ||
| expect(socket).toBeInstanceOf(NodeTCPSocket) | ||
| }) | ||
| }) | ||
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.
Uh oh!
There was an error while loading. Please reload this page.