Host the Hub broker on a foreign listener; drop the shelf endpoint (v1.56.0)

CHANGELOG.md

@@ -1,3 +1,67 @@
+## 1.56.0
+
+The Hub broker can now be hosted on a listener OmnyShell does not own, so another
+Hub — an [OmnyServer](https://pub.dev/packages/omnyserver) Hub, say — can serve
+OmnyShell nodes on its own port, at its own path, beside its own surfaces. One Hub,
+one certificate, one port, two kinds of node.
+
+Nothing changes for a standalone OmnyShell Hub: `OmnyShellHub`'s API and wire
+behaviour are unchanged, and its 808 existing tests pass untouched.
+
+### Added
+
+- **`OmnyShellHubService`** — the broker as an omnyhub `Service`. Mount it on any
+  `OmnyHub` and OmnyShell traffic is routed to it:
+
+  ```dart
+  await hub.registerService(OmnyShellHubService(broker, mount: '/shell'));
+  // nodes then dial wss://host:8443/shell
+  ```
+
+  `HubBroker` was already transport-agnostic — driven by `accept`ing connections —
+  so this is only an adapter; the whole integration is one method. A node needs no
+  change at all: it dials its `hubUri` verbatim, so
+  `omnyshell node start --hub wss://hub:8443/shell` already works.
+
+  The broker authenticates **in band** (it speaks first, with a challenge), so this
+  route must not be given a `ConnectionAuthenticator` — one would consume the very
+  frames the broker is waiting for. Note omnyhub treats a route's `null` connection
+  authenticator as *inherit the hub-wide one*, not *none*.
+
+- **`NodeConfig.home`** overrides the directory a node persists its UID under
+  (`<home>/.omnyshell/node.uid`). The UID file is keyed by the machine, so two node
+  runtimes in one process — an OmnyShell node embedded beside another agent, or
+  several nodes in a test — otherwise contend on the same file and warn about the
+  UID changing under them. Mirrors the existing `gitCredentialsHome` escape hatch.
+
+### Changed
+
+- **`OmnyShellHub` is rebuilt on omnyhub's `OmnyHub`.** It hosts an
+  `OmnyShellHubService` at `/` (which matches every path — a standalone Hub upgrades
+  a WebSocket regardless of what the peer asked for, so existing clients dialling
+  `wss://host:8443` are unaffected).
+  - TLS is now an omnyhub `TlsProvider`: `StaticTls` for a `securityContext`,
+    `ReloadableFileTls` for a `tlsDirectory`. Certificate renewal still rebinds the
+    listener gap-free — established connections drain on the old one while new ones
+    land on the fresh certificate — but omnyhub performs it, and reports it as
+    `TLS certificate renewed` rather than the old `…rebound…`.
+  - The hub's lifecycle, TLS and unhandled-error messages are bridged into
+    `HubConfig.logger`, which would otherwise have gone to omnyhub's default
+    no-op logger and vanished.
+  - Removed `WsServerEndpoint`, the last user of `shelf` — **`shelf` and
+    `shelf_web_socket` are dropped from the dependencies.**
+  - `PemTlsSource` is retained: tunnels are separate raw TCP listeners and still
+    use it.
+
+## 1.55.3
+
+### Changed
+
+- Raise the [omnydrive](https://pub.dev/packages/omnydrive) constraint to `^1.12.2`,
+  which is itself just the same omnyhub `^1.3.0` bump. `^1.12.1` already resolved to
+  it, so this only makes the constraint name the version OmnyShell is built and
+  tested against. No behaviour change and no API change.
+
 ## 1.55.2
 
 ### Changed

lib/omnyshell_hub.dart

@@ -28,10 +28,10 @@ export 'src/application/hub/http_proxy_service.dart';
 export 'src/application/hub/hub_broker.dart';
 export 'src/application/hub/node_registry.dart';
 export 'src/application/hub/omnyshell_hub.dart';
+export 'src/application/hub/omnyshell_hub_service.dart';
 export 'src/application/hub/session_router.dart';
 export 'src/application/hub/tunnel_registry.dart';
 export 'src/infrastructure/auth/authorized_keys_store.dart';
 export 'src/infrastructure/auth/composite_authenticator.dart';
 export 'src/infrastructure/auth/public_key_authenticator.dart';
 export 'src/infrastructure/auth/token_authenticator.dart';
-export 'src/infrastructure/transport/ws_server_endpoint.dart';

lib/src/application/hub/omnyshell_hub.dart

@@ -1,7 +1,7 @@
-import 'dart:async';
 import 'dart:io';
 import 'dart:typed_data';
 
+import 'package:omnyhub/omnyhub.dart' as omnyhub;
 import 'package:tcp_tunnel/tcp_tunnel.dart' show PortRange;
 
 import '../../domain/auth/authenticator.dart';
@@ -13,13 +13,13 @@ import '../../infrastructure/identity/spki.dart';
 import '../../infrastructure/identity/uid_computer.dart';
 import '../../infrastructure/identity/uid_store.dart';
 import '../../infrastructure/tls/pem_tls_source.dart';
-import '../../infrastructure/transport/ws_server_endpoint.dart';
 import '../../shared/utils/clock.dart';
 import '../ai/ai_config.dart';
 import 'audit_log.dart';
 import 'http_proxy_service.dart';
 import 'hub_broker.dart';
 import 'node_registry.dart';
+import 'omnyshell_hub_service.dart';
 import 'session_router.dart';
 
 /// Configuration for an [OmnyShellHub].
@@ -143,16 +143,19 @@ class HubConfig {
 /// final hub = OmnyShellHub(config);
 /// await hub.start();
 /// ```
+///
+/// This is the *standalone* Hub — it owns its listener. To host the broker on a
+/// listener someone else owns (an OmnyServer Hub, say, serving OmnyShell nodes
+/// alongside its own on one port), mount an [OmnyShellHubService] instead.
 class OmnyShellHub {
   /// The hub configuration.
   final HubConfig config;
 
   /// The broker that authenticates, authorizes and relays.
   final HubBroker broker;
 
-  WsServerEndpoint? _endpoint;
+  omnyhub.OmnyHub? _server;
   OmnyUid? _uid;
-  PemTlsSource? _mainTls;
   PemTlsSource? _tunnelTls;
 
   /// Creates a hub from [config].
@@ -184,10 +187,10 @@ class OmnyShellHub {
   OmnyUid? get uid => _uid;
 
   /// The port the hub is listening on (valid after [start]).
-  int get port => _endpoint?.port ?? config.port;
+  int get port => _server?.port ?? config.port;
 
   /// Whether the hub is running.
-  bool get isRunning => _endpoint != null;
+  bool get isRunning => _server != null;
 
   /// A point-in-time snapshot of hub metrics.
   Map<String, dynamic> metrics() => {
@@ -199,61 +202,44 @@ class OmnyShellHub {
 
   /// Binds the TLS endpoint and starts the liveness watchdog.
   Future<void> start() async {
-    if (_endpoint != null) return;
+    if (_server != null) return;
     await _resolveUid();
     _startTunnelTls();
-    broker.start();
-    final dir = config.tlsDirectory;
-    if (dir != null && dir.isNotEmpty) {
-      final source = PemTlsSource(
-        dir,
-        label: 'hub TLS',
-        checkInterval: config.tlsReloadInterval,
-        logger: config.logger,
-        onReloaded: _rebindMain,
-      );
-      source.load();
-      _mainTls = source;
-      _endpoint = await WsServerEndpoint.bind(
-        host: config.host,
-        port: config.port,
-        securityContext: source.context!,
-        onConnection: broker.accept,
-        shared: true,
-      );
-      source.start();
-    } else {
-      _endpoint = await WsServerEndpoint.bind(
-        host: config.host,
-        port: config.port,
-        securityContext: config.securityContext!,
-        onConnection: broker.accept,
-      );
-    }
+
+    // Mounted at '/', which matches every path — a standalone Hub upgrades a
+    // WebSocket regardless of what the peer asked for, and clients dial the bare
+    // authority (`wss://host:8443`).
+    final server = omnyhub.OmnyHub(
+      transports: [
+        omnyhub.HttpTransport.https(
+          address: config.host,
+          port: config.port,
+          tls: _tls(),
+        ),
+      ],
+      // Without this the hub's own lifecycle, TLS-renewal and unhandled-error
+      // messages would go to omnyhub's default NoopLogger and vanish.
+      logger: _BridgeLogger(config.logger),
+      // Drives the certificate re-check: on renewal omnyhub rebinds the listener
+      // gap-free, so established connections drain on the old one while new ones
+      // land on the fresh certificate.
+      tlsRenewalInterval: config.tlsReloadInterval,
+    );
+    // The broker owns its own in-band handshake (it speaks first, with a
+    // challenge), so the route gets no ConnectionAuthenticator.
+    await server.registerService(OmnyShellHubService(broker));
+    await server.start();
+    _server = server;
   }
 
-  /// Re-binds the main listener with a freshly-loaded [ctx] after a certificate
-  /// renewal. Binds the new (shared) listener on the currently-bound port, swaps
-  /// it in, then closes the old one without forcing — established connections
-  /// drain on the old listener while new ones land on the renewed certificate.
-  Future<void> _rebindMain(SecurityContext ctx) async {
-    final old = _endpoint;
-    if (old == null) return;
-    try {
-      _endpoint = await WsServerEndpoint.bind(
-        host: config.host,
-        port: old.port,
-        securityContext: ctx,
-        onConnection: broker.accept,
-        shared: true,
-      );
-      unawaited(old.close(force: false));
-      config.logger?.call('hub TLS listener rebound on renewed certificate');
-    } on Object catch (e) {
-      // Keep serving on the existing listener if the rebind fails.
-      _endpoint = old;
-      config.logger?.call('hub TLS listener rebind failed: $e');
+  /// The TLS provider for the main listener: a static context, or a hot-reloading
+  /// `fullchain.pem`/`privkey.pem` directory.
+  omnyhub.TlsProvider _tls() {
+    final dir = config.tlsDirectory;
+    if (dir != null && dir.isNotEmpty) {
+      return omnyhub.ReloadableFileTls.directory(dir);
     }
+    return omnyhub.StaticTls.context(config.securityContext!);
   }
 
   /// Loads the tunnel TLS certificate from [HubConfig.tunnelTlsDirectory] and
@@ -305,12 +291,32 @@ class OmnyShellHub {
 
   /// Stops the hub and releases the endpoint.
   Future<void> stop({bool force = true}) async {
-    broker.stop();
-    _mainTls?.stop();
-    _mainTls = null;
     _tunnelTls?.stop();
     _tunnelTls = null;
-    await _endpoint?.close(force: force);
-    _endpoint = null;
+    // Stops the transport and then the service, which stops the broker.
+    await _server?.stop(force: force);
+    _server = null;
   }
 }
+
+/// Feeds omnyhub's structured [omnyhub.Logger] into [HubConfig.logger], the
+/// single line-oriented sink OmnyShell configures.
+///
+/// The hosting hub reports its own lifecycle through omnyhub — the listener
+/// binding, a TLS certificate being renewed, an unhandled request error — and
+/// none of it would be visible otherwise.
+class _BridgeLogger with omnyhub.LoggerBase {
+  final void Function(String message)? sink;
+
+  const _BridgeLogger(this.sink);
+
+  @override
+  void log(
+    omnyhub.LogLevel level,
+    String message, {
+    Map<String, Object?> context = const {},
+  }) => sink?.call(context.isEmpty ? message : '$message $context');
+
+  @override
+  omnyhub.Logger child(Map<String, Object?> context) => this;
+}

lib/src/application/hub/omnyshell_hub_service.dart

@@ -0,0 +1,89 @@
+import 'dart:async';
+
+import 'package:omnyhub/omnyhub.dart' as omnyhub;
+
+import '../../infrastructure/transport/frame_connection.dart';
+import '../../protocol/frame_codec.dart';
+import 'hub_broker.dart';
+
+/// The OmnyShell Hub as an omnyhub [omnyhub.Service], so its broker can be
+/// hosted on any [omnyhub.OmnyHub] listener instead of owning one.
+///
+/// This is what lets a *different* Hub — an OmnyServer Hub, say — also serve
+/// OmnyShell nodes: mount this at a path and OmnyShell traffic is routed to it
+/// while the host keeps its own surfaces on the same port and certificate.
+///
+/// ```dart
+/// // On an OmnyServer (or any omnyhub) Hub:
+/// await hub.registerService(OmnyShellHubService(broker, mount: '/shell'));
+/// // Nodes then dial wss://host:8443/shell
+/// ```
+///
+/// [HubBroker] is transport-agnostic — it is driven by `accept`ing connections —
+/// so this class is only an adapter: omnyhub hands over a raw
+/// [omnyhub.Connection], and [FrameConnection] wraps it in OmnyShell's frame
+/// codec.
+///
+/// **The broker must own the socket un-intercepted.** OmnyShell authenticates
+/// *in band*, after the upgrade: the Hub speaks first with a challenge `hello`,
+/// and the peer answers with an `auth.request`. So this route must not be given
+/// an [omnyhub.ConnectionAuthenticator] — one would consume the very frames the
+/// broker is waiting for. Note that on omnyhub a route's `null` authenticator
+/// means *inherit the hub-wide one*, not *none*: a host hub that sets a hub-wide
+/// connection authenticator must ensure it does not reach this mount.
+class OmnyShellHubService extends omnyhub.ServiceBase {
+  /// The broker that authenticates, authorizes and relays.
+  final HubBroker broker;
+
+  /// Builds the frame codec for each accepted connection.
+  final FrameCodec Function()? codecFactory;
+
+  /// Whether this service owns [broker]'s lifecycle.
+  ///
+  /// `true` (the default) starts and stops the broker with the hosting hub. Set
+  /// it `false` when the embedder already drives the broker — as `OmnyShellHub`
+  /// does — so it is not started twice.
+  final bool ownsBroker;
+
+  /// Mounts [broker] as a service.
+  ///
+  /// The default mount is `/`, which matches every path — the behaviour of a
+  /// standalone OmnyShell Hub, whose listener upgrades a WebSocket regardless of
+  /// path. Give it a specific mount (e.g. `/shell`) when sharing a listener.
+  OmnyShellHubService(
+    this.broker, {
+    super.name = 'omnyshell',
+    super.mount = '/',
+    this.codecFactory,
+    this.ownsBroker = true,
+  });
+
+  @override
+  void handleConnection(
+    omnyhub.Connection connection,
+    omnyhub.HubRequest request,
+  ) => broker.accept(
+    FrameConnection.wrap(connection, codec: codecFactory?.call()),
+  );
+
+  /// A plain HTTP request on the mount: this is a WebSocket endpoint, so answer
+  /// with a small status document rather than an upgrade.
+  @override
+  Future<omnyhub.HubResponse> handle(omnyhub.HubRequest request) async =>
+      omnyhub.HubResponse.json({
+        'service': 'omnyshell',
+        'protocol': 'websocket',
+        'hubUid': broker.hubUid,
+        'nodes': broker.registry.all.length,
+      });
+
+  @override
+  Future<void> start() async {
+    if (ownsBroker) broker.start();
+  }
+
+  @override
+  Future<void> stop() async {
+    if (ownsBroker) broker.stop();
+  }
+}

lib/src/application/node/node_runtime.dart

@@ -128,6 +128,16 @@ class NodeConfig {
   /// ([omnyshellHome]). Mainly for tests and multi-node isolation.
   final String? gitCredentialsHome;
 
+  /// Overrides the home directory this node persists its UID under
+  /// (`<home>/.omnyshell/node.uid`). `null` uses the process default
+  /// ([omnyshellHome]).
+  ///
+  /// The UID file is keyed by the *machine*, so two node runtimes in one process
+  /// — an OmnyShell node embedded beside another agent, or several nodes in a
+  /// test — otherwise contend on the same file and warn about the UID changing
+  /// under them. Give each its own home to keep their identities separate.
+  final String? home;
+
   /// Whether a client connection that drops (network loss, crash, terminal
   /// closed) automatically detaches its session — keeping the PTY, shell and
   /// child processes alive for a later resume — instead of terminating it.
@@ -166,6 +176,7 @@ class NodeConfig {
     this.tunnelEnabled = true,
     this.driveRoots = const [],
     this.gitCredentialsHome,
+    this.home,
     this.autoDetachOnDisconnect = true,
     this.autoDetachTimeout,
     this.cleanupInterval = const Duration(minutes: 1),
@@ -250,8 +261,9 @@ class NodeRuntime {
         arch: platform.arch,
         hostname: platform.hostname,
       );
-      final resolution = await const UidStore(
+      final resolution = await UidStore(
         fileName: 'node.uid',
+        home: config.home,
       ).resolve(computed, logger: _log);
       _uid = resolution.uid;
     } on Object catch (e) {