fix: push isolate reuses FGS hub WebSocket (1-socket invariant) (#1104)

* fix: push isolate reuses FGS hub WebSocket instead of opening its own

PushNotificationIsolateManager previously always created a SignalingModuleImpl
directly, opening a second WebSocket when the FGS hub was already running.
This violated the 1-WebSocket invariant.

The fix adds createPushIsolateModule() to SignalingServicePlatform and
WebtritSignalingService. On Android, the method checks IsolateNameServer for
a live FGS hub: if found and acknowledged, a SignalingHubModule is returned
and no new connection is opened. When no hub is active (app killed), falls
back to a direct SignalingModuleImpl.

PushNotificationIsolateManager now exposes an init() method for async hub
discovery, called from _getOrInit() before run(). run() skips connect() when
the module reports isConnected (hub reuse path).

* logs: add diagnostic logs for push isolate signaling module resolution

Adds log lines to verify the 1-socket invariant at runtime:
- _initSignaling(): logs module type and isConnected after createPushIsolateModule
  resolves, making hub-reuse vs direct-socket path visible in logcat
- close(): logs module type and pending request count on teardown
- _getOrInit(): logs before and after init() so init timing is traceable

These logs confirm whether the hub path or fallback path is taken on each push
notification, and that teardown completes cleanly.

* fix: push isolate starts FGS and waits for hub instead of direct socket

Previously createPushIsolateModule fell back immediately to a direct
SignalingModuleImpl when no hub was running (pushBound mode, first push
after app close). This violated the 1-WebSocket invariant: both the push
isolate and the Activity would open their own WebSocket connections.

New flow:
1. Hub already running -> reuse (persistent mode or second push)
2. No hub -> startFgsOnly(pushBound) -> poll IsolateNameServer until hub
   registers and acknowledges -> return SignalingHubModule
3. Hub not available after 10 s -> emergency fallback to direct module

The push isolate now always shares the same FGS WebSocket with the
Activity, matching the intended architecture.

* fix: address Copilot review comments on push isolate hub reuse

- _tryConnectHub: yield to event loop after ack so hub replay events
  (SignalingConnected etc.) are processed before returning the module.
  Prevents run() from calling connect() on an already-live hub session
  due to isConnected being transiently false while replay is in flight.

- run(): throw StateError when called before init() instead of silently
  leaving _completer unresolved and hanging the caller until timeout.

- lefthook.yml: pass {1} to commit-msg-check.sh so the hook reads the
  actual commit message file (git hook argv) instead of git log -1.

* refactor: unify push isolate signaling with Activity via WebtritSignalingService

Push isolate now uses WebtritSignalingService(mode: pushBound) directly,
the same mechanism the Activity uses. HubConnectionManager inside the
service handles FGS start, hub discovery, and auto-reconnect if the hub
is killed between push arrival and Activity open.

Removes createPushIsolateModule from platform API — it was a one-shot
workaround that bypassed HubConnectionManager and had no reconnect logic.
Removes _tryConnectHub, _startFgsOnly, and related constants from the
Android plugin. The single code path eliminates the duplication.

Lifecycle boundary remains unchanged:
  push mode  — FGS lives from push arrival through Activity close
  persistent — FGS lives indefinitely

* fix: address remaining Copilot review comments

- Remove unnecessary async from _initSignaling() — nothing is awaited
- Rename log field hubConnected -> isConnected (accurate with WebtritSignalingService)
- Always call connect() in run() — WebtritSignalingService.connect() is
  idempotent via _startPending/_isConnected guard; the old conditional
  was left over from SignalingHubModule direct usage
- Fix class doc: qualify 1-WebSocket claim as Android-only
- Fix _initSignaling doc: remove auto-reconnect claim, HubConnectionManager
  handles FGS start and hub discovery, not reconnect after disconnect
- commit-msg-check.sh: add guard for missing or unreadable $1 argument

* docs: fix stale comments after signaling unification

- init() doc: hub discovery and FGS start happen in connect() via
  HubConnectionManager, not in init() as the old comment stated
- background_isolate_callbacks: same correction for _getOrInit comment
- commit-msg-check.sh: add set -euo pipefail for consistency with other scripts

* refactor: replace nullable signalingModule with late field and remove unnecessary async from init()

- _signalingModule/_signalingSubscription: SignalingModule? → late SignalingModule;
  eliminates null-aware ?. access throughout the class since run() requires init() first
- Add _initialized bool flag so close() can guard disposal without accessing late fields prematurely
- init(): Future<void> async => _initSignaling() → void init() (sync, no await needed)
- run(): null check replaced with !_initialized guard; remove intermediate `module` local
- All ?. accesses on _signalingModule replaced with direct . access

Author: SERDUN

lefthook.yml

@@ -15,7 +15,7 @@ pre-commit:
 commit-msg:
   commands:
     commit-msg-check:
-      run: bash tool/scripts/commit-msg-check.sh
+      run: bash tool/scripts/commit-msg-check.sh {1}
 
 pre-push:
   parallel: true

lib/features/call/services/background_isolate_callbacks.dart

@@ -47,6 +47,11 @@ Future<PushNotificationIsolateManager> _getOrInit() async {
     certificates: _context!.appCertificates.trustedCertificates,
     logger: Logger('PushNotificationIsolateManager'),
   );
+  // init() constructs WebtritSignalingService and wires up the event subscription.
+  // Hub discovery and FGS start happen in connect(), which is called from run().
+  _logger.info('_getOrInit: initialising signaling module...');
+  _manager!.init();
+  _logger.info('_getOrInit: init complete');
 
   return _manager!;
 }

lib/features/call/services/isolate_manager.dart

@@ -8,7 +8,6 @@ import 'package:webtrit_callkeep/webtrit_callkeep.dart';
 import 'package:webtrit_signaling/webtrit_signaling.dart';
 import 'package:webtrit_signaling_service/webtrit_signaling_service.dart';
 
-import 'package:webtrit_phone/app/constants.dart';
 import 'package:webtrit_phone/data/data.dart';
 import 'package:webtrit_phone/models/models.dart';
 import 'package:webtrit_phone/repositories/repositories.dart';
@@ -22,6 +21,10 @@ import '../models/jsep_value.dart';
 /// to retrieve call state, handles missed-call logging and notifications, and
 /// releases the incoming call service when all work is done.
 /// Never reconnects — the isolate is short-lived by design.
+///
+/// On Android, signaling runs through the FGS hub so push isolate and Activity
+/// share a single WebSocket connection. On iOS the connection runs directly in
+/// the main isolate. Call [init] after construction and before [run].
 class PushNotificationIsolateManager implements CallkeepBackgroundServiceDelegate {
   PushNotificationIsolateManager({
     required this.callLogsRepository,
@@ -31,7 +34,9 @@ class PushNotificationIsolateManager implements CallkeepBackgroundServiceDelegat
     required this.certificates,
     required this.logger,
   }) : _pushService = callkeep {
-    _initSignaling();
+    // setBackgroundServiceDelegate is called in the constructor so callkeep can
+    // route performAnswerCall / performEndCall as soon as the object exists,
+    // before [init] is called.
     _pushService.setBackgroundServiceDelegate(this);
   }
 
@@ -43,8 +48,10 @@ class PushNotificationIsolateManager implements CallkeepBackgroundServiceDelegat
 
   final BackgroundPushNotificationService _pushService;
 
-  late final SignalingModule _signalingModule;
-  late final StreamSubscription<SignalingModuleEvent> _signalingSubscription;
+  // Assigned exactly once in [init], before any call to [run] or [close].
+  late SignalingModule _signalingModule;
+  late StreamSubscription<SignalingModuleEvent> _signalingSubscription;
+  bool _initialized = false;
 
   /// Metadata from the incoming push notification.
   /// Used as a fallback for missed-call display name and call logging.
@@ -69,28 +76,51 @@ class PushNotificationIsolateManager implements CallkeepBackgroundServiceDelegat
   // Public API
   // ---------------------------------------------------------------------------
 
+  /// Initialises the signaling module.
+  ///
+  /// Must be called once after construction and before [run]. Constructs
+  /// [WebtritSignalingService] and wires up the event subscription. Hub
+  /// discovery and FGS start happen later when [connect] is called from
+  /// [run] via the Android plugin's [HubConnectionManager].
+  void init() {
+    _initSignaling();
+    _initialized = true;
+  }
+
   /// Connects to the signaling server, processes call state for the given push
   /// notification [metadata], and returns a [Future] that completes after all
   /// work is done (notifications shown, logs written, native service released).
   Future<void> run(CallkeepIncomingCallMetadata? metadata) {
+    if (!_initialized) {
+      throw StateError('PushNotificationIsolateManager.run() called before init()');
+    }
     _metadata = metadata;
     _completer = Completer<void>();
-    logger.info('run: callId=${metadata?.callId}');
+    logger.info('run: callId=${metadata?.callId} isConnected=${_signalingModule.isConnected}');
+    // WebtritSignalingService.connect() is idempotent: the internal
+    // _startPending / _isConnected guard makes repeated calls safe.
+    // Always call it so HubConnectionManager starts FGS discovery on the
+    // first run() and is a no-op on any subsequent call.
     _signalingModule.connect();
     return _completer!.future;
   }
 
   /// Cancels all timers and pending requests, then disposes the signaling module.
   Future<void> close() async {
+    logger.info(
+      'close: disposing module=${_initialized ? _signalingModule.runtimeType : "not initialized"} pendingRequests=${_pendingRequests.length}',
+    );
     for (final pending in _pendingRequests) {
       pending.timeoutTimer.cancel();
       if (!pending.completer.isCompleted) {
         pending.completer.completeError(StateError('PushNotificationIsolateManager closed'));
       }
     }
     _pendingRequests.clear();
-    await _signalingSubscription.cancel();
-    await _signalingModule.dispose();
+    if (_initialized) {
+      await _signalingSubscription.cancel();
+      await _signalingModule.dispose();
+    }
     await _releaseCall(_metadata?.callId);
     _completeWithError(StateError('PushNotificationIsolateManager closed'));
   }
@@ -122,14 +152,22 @@ class PushNotificationIsolateManager implements CallkeepBackgroundServiceDelegat
   // Signaling init
   // ---------------------------------------------------------------------------
 
+  /// Sets up [WebtritSignalingService] for this isolate in
+  /// [SignalingServiceMode.pushBound] mode — the same mechanism the Activity
+  /// uses, so push isolate and Activity share exactly one FGS WebSocket on
+  /// Android. [HubConnectionManager] inside the service handles FGS start and
+  /// hub discovery. [connect] is called from [run], not here, so the
+  /// connection starts only when processing begins.
   void _initSignaling() {
-    _signalingModule = SignalingModuleImpl(
-      coreUrl: storage.readCoreUrl() ?? '',
-      tenantId: storage.readTenantId() ?? '',
-      token: storage.readToken() ?? '',
-      trustedCertificates: certificates,
-      connectionTimeout: kSignalingClientConnectionTimeout,
-      reconnectDelay: kSignalingClientReconnectDelay,
+    logger.info('_initSignaling: creating WebtritSignalingService (pushBound)');
+    _signalingModule = WebtritSignalingService(
+      config: SignalingServiceConfig(
+        coreUrl: storage.readCoreUrl() ?? '',
+        tenantId: storage.readTenantId() ?? '',
+        token: storage.readToken() ?? '',
+        trustedCertificates: certificates,
+      ),
+      mode: SignalingServiceMode.pushBound,
     );
 
     _signalingSubscription = _signalingModule.events.listen((event) {

tool/scripts/commit-msg-check.sh

@@ -1,6 +1,12 @@
 #!/bin/bash
+set -euo pipefail
 
-MSG=$(git log -1 --pretty=%B | head -n1)
+if [[ -z "$1" || ! -f "$1" ]]; then
+  echo "❌ Usage: commit-msg-check.sh <commit-msg-file>"
+  exit 1
+fi
+
+MSG=$(head -n1 "$1")
 
 if ! [[ "$MSG" =~ ^(feat|fix|chore|refactor|test|docs|style|ci|perf|build|revert)(\(.+\))?:\ .+ ]]; then
   echo "❌ Invalid commit message: '$MSG'"