fix: address review findings — dedup helpers, fix tests, fix error handler

- Fix uncaughtException handler to use console.error + exitCode instead
  of re-throwing (which hard-kills the process without Vitest diagnostics)
- Add missing IPv6 test case for getHost in multiaddr.test.ts
- Add missing empty-string edge case for isPrivateAddress in network.test.ts
- Extract stopWithTimeout to test/helpers/stop-with-timeout.ts (was
  duplicated across 3 e2e test files)
- Extract getRemoteCommsPeerId to test/helpers/remote-comms.ts (was
  duplicated in relay-connectivity and bip39 tests)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Author: sirtimid

packages/kernel-node-runtime/test/e2e/bip39-identity-recovery.test.ts

@@ -1,12 +1,12 @@
 import { makeSQLKernelDatabase } from '@metamask/kernel-store/sqlite/nodejs';
 import { Kernel } from '@metamask/ocap-kernel';
-import type { KernelStatus } from '@metamask/ocap-kernel';
 import { mkdtemp, rm } from 'node:fs/promises';
 import { tmpdir } from 'node:os';
 import { join } from 'node:path';
 import { describe, it, expect } from 'vitest';
 
 import { makeTestKernel } from '../helpers/kernel.ts';
+import { getRemoteCommsPeerId } from '../helpers/remote-comms.ts';
 
 // Valid 12-word test mnemonic (DO NOT use in production)
 const TEST_MNEMONIC =
@@ -23,21 +23,6 @@ const DUMMY_RELAYS = [
   '/ip4/127.0.0.1/tcp/19001/ws/p2p/12D3KooWBTf3S95YAsh6fi5rhohMsk8qaTy19rrpW8X9G69vn6GP',
 ];
 
-/**
- * Extract peerId from remoteComms status, returning undefined for disconnected state.
- *
- * @param remoteComms - The remote comms status object.
- * @returns The peer ID string or undefined.
- */
-function getRemoteCommsPeerId(
-  remoteComms: KernelStatus['remoteComms'],
-): string | undefined {
-  if (remoteComms && remoteComms.state !== 'disconnected') {
-    return remoteComms.peerId;
-  }
-  return undefined;
-}
-
 // Tests for identity recovery using mnemonic
 // Note: These tests verify that the same mnemonic produces the same peer ID
 // The peer ID is derived locally from the mnemonic

packages/kernel-node-runtime/test/e2e/quic-transport.test.ts

@@ -10,34 +10,11 @@ import {
   makeRemoteVatConfig,
   sendRemoteMessage,
 } from '../helpers/remote-comms.ts';
+import { stopWithTimeout } from '../helpers/stop-with-timeout.ts';
 
 // Increase timeout for network operations
 const NETWORK_TIMEOUT = 30_000;
 
-/**
- * Stop an operation with a timeout to prevent hangs during cleanup.
- *
- * @param stopFn - The stop function to call.
- * @param timeoutMs - The timeout in milliseconds.
- * @param label - A label for logging.
- */
-async function stopWithTimeout(
-  stopFn: () => Promise<unknown>,
-  timeoutMs: number,
-  label: string,
-): Promise<void> {
-  try {
-    await Promise.race([
-      stopFn(),
-      new Promise<never>((_resolve, reject) =>
-        setTimeout(() => reject(new Error(`${label} timed out`)), timeoutMs),
-      ),
-    ]);
-  } catch {
-    // Ignore timeout errors during cleanup
-  }
-}
-
 // Listen addresses for each kernel (port 0 = OS-assigned)
 const quicListenAddress = '/ip4/127.0.0.1/udp/0/quic-v1';
 const tcpListenAddress = '/ip4/127.0.0.1/tcp/0';

packages/kernel-node-runtime/test/e2e/relay-connectivity.test.ts

@@ -2,50 +2,18 @@ import type { Libp2p } from '@libp2p/interface';
 import { makeSQLKernelDatabase } from '@metamask/kernel-store/sqlite/nodejs';
 import { startRelay } from '@metamask/kernel-utils/libp2p';
 import { Kernel } from '@metamask/ocap-kernel';
-import type { KernelStatus } from '@metamask/ocap-kernel';
 import { createConnection } from 'node:net';
 import { describe, it, expect, beforeAll, afterAll } from 'vitest';
 
 import { makeTestKernel } from '../helpers/kernel.ts';
+import { getRemoteCommsPeerId } from '../helpers/remote-comms.ts';
+import { stopWithTimeout } from '../helpers/stop-with-timeout.ts';
 
 const STOP_TIMEOUT = 5_000;
 const TEST_TIMEOUT = 30_000;
 const RELAY_PEER_ID = '12D3KooWJBDqsyHQF2MWiCdU4kdqx4zTsSTLRdShg7Ui6CRWB4uc';
 const RELAY_WS_PORT = 9001;
 
-/**
- * Stop an operation with a timeout to prevent hangs during cleanup.
- *
- * @param stopFn - The stop function to call.
- * @param timeoutMs - The timeout in milliseconds.
- * @param label - A label for logging.
- */
-async function stopWithTimeout(
-  stopFn: () => Promise<unknown>,
-  timeoutMs: number,
-  label: string,
-): Promise<void> {
-  try {
-    await Promise.race([
-      stopFn(),
-      new Promise<never>((_resolve, reject) =>
-        setTimeout(() => reject(new Error(`${label} timed out`)), timeoutMs),
-      ),
-    ]);
-  } catch {
-    // Ignore timeout errors during cleanup
-  }
-}
-
-function getRemoteCommsPeerId(
-  remoteComms: KernelStatus['remoteComms'],
-): string | undefined {
-  if (remoteComms && remoteComms.state !== 'disconnected') {
-    return remoteComms.peerId;
-  }
-  return undefined;
-}
-
 /**
  * Check if a TCP port is already in use.
  *

packages/kernel-node-runtime/test/e2e/remote-comms.test.ts

@@ -22,33 +22,10 @@ import {
   sendRemoteMessage,
   setupAliceAndBob,
 } from '../helpers/remote-comms.ts';
+import { stopWithTimeout } from '../helpers/stop-with-timeout.ts';
 
 // Increase timeout for network operations
 const NETWORK_TIMEOUT = 30_000;
-
-/**
- * Stop an operation with a timeout to prevent hangs during cleanup.
- *
- * @param stopFn - The stop function to call.
- * @param timeoutMs - The timeout in milliseconds.
- * @param label - A label for logging.
- */
-async function stopWithTimeout(
-  stopFn: () => Promise<unknown>,
-  timeoutMs: number,
-  label: string,
-): Promise<void> {
-  try {
-    await Promise.race([
-      stopFn(),
-      new Promise<never>((_resolve, reject) =>
-        setTimeout(() => reject(new Error(`${label} timed out`)), timeoutMs),
-      ),
-    ]);
-  } catch {
-    // Ignore timeout errors during cleanup
-  }
-}
 // Test relay configuration
 // The relay peer ID is deterministic based on RELAY_LOCAL_ID = 200 in relay.ts
 const relayPeerId = '12D3KooWJBDqsyHQF2MWiCdU4kdqx4zTsSTLRdShg7Ui6CRWB4uc';

packages/kernel-node-runtime/test/e2e/setup-suppress-datachannel-error.ts

@@ -17,6 +17,9 @@ process.on('uncaughtException', (error: Error) => {
     // Benign: native WebRTC module fires async error after channel teardown.
     return;
   }
-  // Re-throw anything else so Vitest still catches real errors.
-  throw error;
+  // For non-datachannel errors, log and set exit code so Vitest reports
+  // the failure. Re-throwing inside uncaughtException would hard-kill the
+  // process without Vitest diagnostics.
+  console.error('Uncaught exception in e2e test:', error);
+  process.exitCode = 1;
 });

packages/kernel-node-runtime/test/helpers/remote-comms.ts

@@ -4,11 +4,28 @@ import { Kernel, kunser, makeKernelStore } from '@metamask/ocap-kernel';
 import type {
   ClusterConfig,
   KRef,
+  KernelStatus,
   RemoteCommsOptions,
 } from '@metamask/ocap-kernel';
 
 import { makeTestKernel } from './kernel.ts';
 
+/**
+ * Extract peerId from remoteComms status, returning undefined for
+ * disconnected state.
+ *
+ * @param remoteComms - The remote comms status object.
+ * @returns The peer ID string or undefined.
+ */
+export function getRemoteCommsPeerId(
+  remoteComms: KernelStatus['remoteComms'],
+): string | undefined {
+  if (remoteComms && remoteComms.state !== 'disconnected') {
+    return remoteComms.peerId;
+  }
+  return undefined;
+}
+
 /**
  * Helper to create a vat configuration for a remote vat.
  *

packages/kernel-node-runtime/test/helpers/stop-with-timeout.ts

@@ -0,0 +1,23 @@
+/**
+ * Stop an operation with a timeout to prevent hangs during cleanup.
+ *
+ * @param stopFn - The stop function to call.
+ * @param timeoutMs - The timeout in milliseconds.
+ * @param label - A label for logging.
+ */
+export async function stopWithTimeout(
+  stopFn: () => Promise<unknown>,
+  timeoutMs: number,
+  label: string,
+): Promise<void> {
+  try {
+    await Promise.race([
+      stopFn(),
+      new Promise<never>((_resolve, reject) =>
+        setTimeout(() => reject(new Error(`${label} timed out`)), timeoutMs),
+      ),
+    ]);
+  } catch {
+    // Ignore timeout errors during cleanup
+  }
+}

packages/ocap-kernel/src/utils/multiaddr.test.ts

@@ -37,6 +37,11 @@ describe('getHost', () => {
       addr: '/dns4/relay.example.com/tcp/443/wss',
       expected: 'relay.example.com',
     },
+    {
+      desc: 'extracts IPv6 host',
+      addr: '/ip6/::1/tcp/9001/ws',
+      expected: '::1',
+    },
     {
       desc: 'extracts dns6 host',
       addr: '/dns6/relay.example.com/tcp/443/wss',

packages/ocap-kernel/src/utils/network.test.ts

@@ -47,6 +47,9 @@ describe('isPrivateAddress', () => {
     { host: 'relay.example.com', expected: false },
     { host: 'google.com', expected: false },
 
+    // Empty string (from getHost() ?? '' fallback)
+    { host: '', expected: false },
+
     // Invalid IPv4 (octets > 255)
     { host: '256.0.0.1', expected: false },
   ])('isPrivateAddress("$host") → $expected', ({ host, expected }) => {