diff --git a/packages/kernel-browser-runtime/src/PlatformServicesClient.test.ts b/packages/kernel-browser-runtime/src/PlatformServicesClient.test.ts index e34390c4e..e4c1d1243 100644 --- a/packages/kernel-browser-runtime/src/PlatformServicesClient.test.ts +++ b/packages/kernel-browser-runtime/src/PlatformServicesClient.test.ts @@ -108,19 +108,72 @@ describe('PlatformServicesClient', () => { await expect(resultP).rejects.toThrow('foo'); }); - it('calls logger.debug when receiving an unexpected reply', async () => { - const debugSpy = vi.spyOn(clientLogger, 'debug'); - const unexpectedReply = makeNullReply('m9'); + it('calls logger.error when receiving response with non-string id', async () => { + const errorSpy = vi.spyOn(clientLogger, 'error'); + const unexpectedReply = new MessageEvent('message', { + data: { + id: 123, + result: null, + jsonrpc: '2.0', + }, + }); await stream.receiveInput(unexpectedReply); await delay(10); - expect(debugSpy).toHaveBeenCalledOnce(); - expect(debugSpy).toHaveBeenLastCalledWith( - 'Received response with unexpected id "m9".', + expect(errorSpy).toHaveBeenCalledOnce(); + expect(errorSpy).toHaveBeenLastCalledWith( + 'Received response with unexpected id:', + expect.any(String), + ); + }); + + it('calls logger.error when receiving message with unexpected port', async () => { + const errorSpy = vi.spyOn(clientLogger, 'error'); + const unexpectedPortReply = makeMessageEvent( + 'm9', + { result: null }, + new MessageChannel().port1, + ); + + await stream.receiveInput(unexpectedPortReply); + await delay(10); + + expect(errorSpy).toHaveBeenCalledOnce(); + expect(errorSpy).toHaveBeenLastCalledWith( + 'Received message with unexpected port:', + expect.any(String), ); }); + it('handles request with unknown method by sending error response', async () => { + const outputs: MessageEventWithPayload[] = []; + const newStream = await TestDuplexStream.make((message) => { + outputs.push(message as unknown as MessageEventWithPayload); + }); + // eslint-disable-next-line no-new -- test setup + new PlatformServicesClient( + newStream as unknown as PlatformServicesClientStream, + ); + + await newStream.receiveInput( + new MessageEvent('message', { + data: { + id: 'm1', + jsonrpc: '2.0', + method: 'unknownMethod', + params: {}, + }, + }), + ); + await delay(10); + + const errorResponse = outputs.find( + (message) => message.payload?.id === 'm1' && 'error' in message.payload, + ); + expect(errorResponse).toBeDefined(); + }); + describe('launch', () => { it('resolves with a duplex stream when receiving a launch reply', async () => { const vatId: VatId = 'v0'; @@ -191,7 +244,50 @@ describe('PlatformServicesClient', () => { const remoteHandler = vi.fn(async () => 'response'); const result = client.initializeRemoteComms( '0xabcd', - ['/dns4/relay.example/tcp/443/wss/p2p/relayPeer'], + { relays: ['/dns4/relay.example/tcp/443/wss/p2p/relayPeer'] }, + remoteHandler, + ); + await stream.receiveInput(makeNullReply('m1')); + expect(await result).toBeUndefined(); + }); + + it('sends initializeRemoteComms with all options and resolves', async () => { + const remoteHandler = vi.fn(async () => 'response'); + const result = client.initializeRemoteComms( + '0xabcd', + { + relays: ['/dns4/relay.example/tcp/443/wss/p2p/relayPeer'], + maxRetryAttempts: 5, + maxQueue: 100, + }, + remoteHandler, + ); + await stream.receiveInput(makeNullReply('m1')); + expect(await result).toBeUndefined(); + }); + + it('sends initializeRemoteComms with onRemoteGiveUp callback', async () => { + const remoteHandler = vi.fn(async () => 'response'); + const giveUpHandler = vi.fn(); + const result = client.initializeRemoteComms( + '0xabcd', + { relays: ['/dns4/relay.example/tcp/443/wss/p2p/relayPeer'] }, + remoteHandler, + giveUpHandler, + ); + await stream.receiveInput(makeNullReply('m1')); + expect(await result).toBeUndefined(); + }); + + it('filters undefined values from options', async () => { + const remoteHandler = vi.fn(async () => 'response'); + const result = client.initializeRemoteComms( + '0xabcd', + { + relays: ['/dns4/relay.example/tcp/443/wss/p2p/relayPeer'], + maxRetryAttempts: undefined, + maxQueue: undefined, + }, remoteHandler, ); await stream.receiveInput(makeNullReply('m1')); @@ -283,6 +379,138 @@ describe('PlatformServicesClient', () => { ); expect(errorResponse).toBeDefined(); }); + + it('calls handler and returns response when handler is set', async () => { + const outputs: MessageEventWithPayload[] = []; + const testStream = await TestDuplexStream.make((message) => { + outputs.push(message as unknown as MessageEventWithPayload); + }); + const testClient = new PlatformServicesClient( + testStream as unknown as PlatformServicesClientStream, + clientLogger, + ); + // Wait for client to be ready + await delay(10); + + const remoteHandler = vi.fn(async () => 'response-message'); + const initP = testClient.initializeRemoteComms( + '0xabcd', + {}, + remoteHandler, + ); + await testStream.receiveInput(makeNullReply('m1')); + await initP; + + await testStream.receiveInput( + new MessageEvent('message', { + data: { + id: 'm2', + jsonrpc: '2.0', + method: 'remoteDeliver', + params: { + from: 'peer-123', + message: 'test-message', + }, + }, + }), + ); + await delay(50); + + expect(remoteHandler).toHaveBeenCalledOnce(); + expect(remoteHandler).toHaveBeenCalledWith( + 'peer-123', + 'test-message', + ); + + const successResponse = outputs.find( + (message) => + message.payload?.id === 'm2' && + 'result' in message.payload && + message.payload.result === 'response-message', + ); + expect(successResponse).toBeDefined(); + }); + }); + + describe('remoteGiveUp', () => { + it('calls handler when handler is set', async () => { + const outputs: MessageEventWithPayload[] = []; + const testStream = await TestDuplexStream.make((message) => { + outputs.push(message as unknown as MessageEventWithPayload); + }); + const testClient = new PlatformServicesClient( + testStream as unknown as PlatformServicesClientStream, + clientLogger, + ); + // Wait for client to be ready + await delay(10); + + const remoteHandler = vi.fn(async () => 'response'); + const giveUpHandler = vi.fn(); + const initP = testClient.initializeRemoteComms( + '0xabcd', + {}, + remoteHandler, + giveUpHandler, + ); + await testStream.receiveInput(makeNullReply('m1')); + await initP; + + await testStream.receiveInput( + new MessageEvent('message', { + data: { + id: 'm2', + jsonrpc: '2.0', + method: 'remoteGiveUp', + params: { peerId: 'peer-456' }, + }, + }), + ); + await delay(50); + + expect(giveUpHandler).toHaveBeenCalledOnce(); + expect(giveUpHandler).toHaveBeenCalledWith('peer-456'); + + const successResponse = outputs.find( + (message) => + message.payload?.id === 'm2' && + 'result' in message.payload && + message.payload.result === null, + ); + expect(successResponse).toBeDefined(); + }); + + it('does not throw when handler is not set', async () => { + const outputs: MessageEventWithPayload[] = []; + const newStream = await TestDuplexStream.make((message) => { + outputs.push(message as unknown as MessageEventWithPayload); + }); + // eslint-disable-next-line no-new -- test setup + new PlatformServicesClient( + newStream as unknown as PlatformServicesClientStream, + ); + + await newStream.receiveInput( + new MessageEvent('message', { + data: { + id: 'm1', + jsonrpc: '2.0', + method: 'remoteGiveUp', + params: { peerId: 'peer-789' }, + }, + }), + ); + await delay(10); + + // Should have sent success response with null result + const successResponse = outputs.find( + (message) => + message.payload?.id === 'm1' && + 'result' in message.payload && + message.payload.result === null, + ); + expect(successResponse).toBeDefined(); + }); }); }); }); diff --git a/packages/kernel-browser-runtime/src/PlatformServicesClient.ts b/packages/kernel-browser-runtime/src/PlatformServicesClient.ts index 6fb4c7be2..9bfbac399 100644 --- a/packages/kernel-browser-runtime/src/PlatformServicesClient.ts +++ b/packages/kernel-browser-runtime/src/PlatformServicesClient.ts @@ -7,6 +7,7 @@ import type { RemoteMessageHandler, VatId, VatConfig, + RemoteCommsOptions, } from '@metamask/ocap-kernel'; import { platformServicesMethodSpecs, @@ -47,6 +48,8 @@ export class PlatformServicesClient implements PlatformServices { #remoteMessageHandler: RemoteMessageHandler | undefined = undefined; + #remoteGiveUpHandler: ((peerId: string) => void) | undefined = undefined; + /** * **ATTN:** Prefer {@link PlatformServicesClient.make} over constructing * this class directly. @@ -80,6 +83,7 @@ export class PlatformServicesClient implements PlatformServices { ); this.#rpcServer = new RpcService(kernelRemoteHandlers, { remoteDeliver: this.#remoteDeliver.bind(this), + remoteGiveUp: this.#remoteGiveUp.bind(this), }); // Start draining messages immediately after construction @@ -171,21 +175,29 @@ export class PlatformServicesClient implements PlatformServices { * Initialize network communications. * * @param keySeed - The seed for generating this kernel's secret key. - * @param knownRelays - Array of the peerIDs of relay nodes that can be used to listen for incoming + * @param options - Options for remote communications initialization. + * @param options.relays - Array of the peerIDs of relay nodes that can be used to listen for incoming * connections from other kernels. + * @param options.maxRetryAttempts - Maximum number of reconnection attempts. 0 = infinite (default). + * @param options.maxQueue - Maximum number of messages to queue per peer while reconnecting (default: 200). * @param remoteMessageHandler - A handler function to receive remote messages. + * @param onRemoteGiveUp - Optional callback to be called when we give up on a remote. * @returns A promise that resolves once network access has been established * or rejects if there is some problem doing so. */ async initializeRemoteComms( keySeed: string, - knownRelays: string[], + options: RemoteCommsOptions, remoteMessageHandler: (from: string, message: string) => Promise, + onRemoteGiveUp?: (peerId: string) => void, ): Promise { this.#remoteMessageHandler = remoteMessageHandler; + this.#remoteGiveUpHandler = onRemoteGiveUp; await this.#rpcClient.call('initializeRemoteComms', { keySeed, - knownRelays, + ...Object.fromEntries( + Object.entries(options).filter(([, value]) => value !== undefined), + ), }); } @@ -252,6 +264,19 @@ export class PlatformServicesClient implements PlatformServices { throw Error(`remote message handler not set`); } + /** + * Handle a remote give up notification from the server. + * + * @param peerId - The peer ID of the remote we're giving up on. + * @returns A promise that resolves when handling is complete. + */ + async #remoteGiveUp(peerId: string): Promise { + if (this.#remoteGiveUpHandler) { + this.#remoteGiveUpHandler(peerId); + } + return null; + } + /** * Send a message to the server. * diff --git a/packages/kernel-browser-runtime/src/PlatformServicesServer.test.ts b/packages/kernel-browser-runtime/src/PlatformServicesServer.test.ts index c5366b67e..b27b6b5e7 100644 --- a/packages/kernel-browser-runtime/src/PlatformServicesServer.test.ts +++ b/packages/kernel-browser-runtime/src/PlatformServicesServer.test.ts @@ -23,6 +23,10 @@ const mockSendRemoteMessage = vi.fn(async () => undefined); const mockStop = vi.fn(async () => undefined); const mockCloseConnection = vi.fn(async () => undefined); const mockReconnectPeer = vi.fn(async () => undefined); +let capturedRemoteMessageHandler: + | ((from: string, message: string) => Promise) + | undefined; +let capturedRemoteGiveUpHandler: ((peerId: string) => void) | undefined; vi.mock('@metamask/ocap-kernel', () => ({ PlatformServicesCommandMethod: { @@ -30,12 +34,23 @@ vi.mock('@metamask/ocap-kernel', () => ({ terminate: 'terminate', terminateAll: 'terminateAll', }, - initNetwork: vi.fn(async () => ({ - sendRemoteMessage: mockSendRemoteMessage, - stop: mockStop, - closeConnection: mockCloseConnection, - reconnectPeer: mockReconnectPeer, - })), + initNetwork: vi.fn( + async ( + _keySeed: string, + _options: unknown, + remoteMessageHandler: (from: string, message: string) => Promise, + remoteGiveUpHandler: (peerId: string) => void, + ) => { + capturedRemoteMessageHandler = remoteMessageHandler; + capturedRemoteGiveUpHandler = remoteGiveUpHandler; + return { + sendRemoteMessage: mockSendRemoteMessage, + stop: mockStop, + closeConnection: mockCloseConnection, + reconnectPeer: mockReconnectPeer, + }; + }, + ), })); const makeVatConfig = (sourceSpec = 'bogus.js'): VatConfig => ({ @@ -78,11 +93,11 @@ const makeTerminateAllMessageEvent = (messageId: `m${number}`): MessageEvent => const makeInitializeRemoteCommsMessageEvent = ( messageId: `m${number}`, keySeed: string, - knownRelays: string[], + options: { relays?: string[]; maxRetryAttempts?: number; maxQueue?: number }, ): MessageEvent => makeMessageEvent(messageId, { method: 'initializeRemoteComms', - params: { keySeed, knownRelays }, + params: { keySeed, ...options }, }); const makeSendRemoteMessageMessageEvent = ( @@ -212,6 +227,36 @@ describe('PlatformServicesServer', () => { ); }); + it('handles JsonRpcResponse messages', async () => { + const outputs: unknown[] = []; + const testStream = await TestDuplexStream.make((message) => { + outputs.push(message); + }); + await testStream.synchronize(); + // eslint-disable-next-line no-new + new PlatformServicesServer( + testStream as unknown as PlatformServicesStream, + makeMockVatWorker, + logger, + ); + + // Send a response (simulating RPC client response) + await testStream.receiveInput( + new MessageEvent('message', { + data: { + id: 'vws:1', + result: 'test-result', + jsonrpc: '2.0', + }, + }), + ); + await delay(10); + + // Response should be handled without errors + // (RPC client handles it internally) + expect(testStream).toBeDefined(); + }); + describe('launch', () => { it('launches a vat', async () => { const vatId = 'v0'; @@ -339,22 +384,47 @@ describe('PlatformServicesServer', () => { mockStop.mockClear(); mockCloseConnection.mockClear(); mockReconnectPeer.mockClear(); + capturedRemoteMessageHandler = undefined; + capturedRemoteGiveUpHandler = undefined; }); describe('initializeRemoteComms', () => { it('initializes remote comms with keySeed and relays', async () => { const keySeed = '0x1234567890abcdef'; - const knownRelays = ['/dns4/relay.example/tcp/443/wss/p2p/relayPeer']; + const relays = ['/dns4/relay.example/tcp/443/wss/p2p/relayPeer']; await stream.receiveInput( - makeInitializeRemoteCommsMessageEvent('m0', keySeed, knownRelays), + makeInitializeRemoteCommsMessageEvent('m0', keySeed, { relays }), ); await delay(10); const { initNetwork } = await import('@metamask/ocap-kernel'); expect(initNetwork).toHaveBeenCalledWith( keySeed, - knownRelays, + { relays }, + expect.any(Function), + expect.any(Function), + ); + }); + + it('initializes remote comms with all options', async () => { + const keySeed = '0x1234567890abcdef'; + const options = { + relays: ['/dns4/relay.example/tcp/443/wss/p2p/relayPeer'], + maxRetryAttempts: 5, + maxQueue: 100, + }; + + await stream.receiveInput( + makeInitializeRemoteCommsMessageEvent('m0', keySeed, options), + ); + await delay(10); + + const { initNetwork } = await import('@metamask/ocap-kernel'); + expect(initNetwork).toHaveBeenCalledWith( + keySeed, + options, + expect.any(Function), expect.any(Function), ); }); @@ -362,17 +432,17 @@ describe('PlatformServicesServer', () => { it('throws error if already initialized', async () => { const errorSpy = vi.spyOn(logger, 'error'); const keySeed = '0xabcd'; - const knownRelays = ['/dns4/relay.example/tcp/443/wss/p2p/relayPeer']; + const relays = ['/dns4/relay.example/tcp/443/wss/p2p/relayPeer']; // First initialization await stream.receiveInput( - makeInitializeRemoteCommsMessageEvent('m0', keySeed, knownRelays), + makeInitializeRemoteCommsMessageEvent('m0', keySeed, { relays }), ); await delay(10); // Second initialization should fail await stream.receiveInput( - makeInitializeRemoteCommsMessageEvent('m1', keySeed, knownRelays), + makeInitializeRemoteCommsMessageEvent('m1', keySeed, { relays }), ); await delay(10); @@ -385,13 +455,138 @@ describe('PlatformServicesServer', () => { }); }); + describe('handleRemoteMessage', () => { + it('captures handler from initNetwork', async () => { + const keySeed = '0xabcd'; + const relays = ['/dns4/relay.example/tcp/443/wss/p2p/relayPeer']; + + await stream.receiveInput( + makeInitializeRemoteCommsMessageEvent('m0', keySeed, { relays }), + ); + await delay(10); + + // Handler should be captured + expect(capturedRemoteMessageHandler).toBeDefined(); + expect(typeof capturedRemoteMessageHandler).toBe('function'); + }); + + it('sends RPC call for remoteDeliver when handler is called', async () => { + const keySeed = '0xabcd'; + const relays = ['/dns4/relay.example/tcp/443/wss/p2p/relayPeer']; + + // Capture RPC calls through stream + const outputs: unknown[] = []; + const testStream = await TestDuplexStream.make((message) => { + outputs.push(message); + }); + await testStream.synchronize(); + // eslint-disable-next-line no-new + new PlatformServicesServer( + testStream as unknown as PlatformServicesStream, + makeMockVatWorker, + logger, + ); + await testStream.receiveInput( + makeInitializeRemoteCommsMessageEvent('m1', keySeed, { relays }), + ); + await delay(10); + + // Handler should be captured and callable + expect(capturedRemoteMessageHandler).toBeDefined(); + expect(typeof capturedRemoteMessageHandler).toBe('function'); + + // Call handler - it will send RPC request + const handlerPromise = capturedRemoteMessageHandler?.( + 'peer-123', + 'test-message', + ); + + await delay(10); + + // Should have sent remoteDeliver RPC request + const remoteDeliverCall = outputs.find((outputMessage: unknown) => { + const parsedMessage = outputMessage as { + payload?: { method?: string }; + }; + return parsedMessage.payload?.method === 'remoteDeliver'; + }); + expect(remoteDeliverCall).toBeDefined(); + + // Mock response to complete the handler call + await testStream.receiveInput( + new MessageEvent('message', { + data: { + id: 'vws:1', + result: '', + jsonrpc: '2.0', + }, + }), + ); + await handlerPromise; + }); + }); + + describe('handleRemoteGiveUp', () => { + it('captures handler from initNetwork', async () => { + const keySeed = '0xabcd'; + const relays = ['/dns4/relay.example/tcp/443/wss/p2p/relayPeer']; + + await stream.receiveInput( + makeInitializeRemoteCommsMessageEvent('m0', keySeed, { relays }), + ); + await delay(10); + + // Handler should be captured + expect(capturedRemoteGiveUpHandler).toBeDefined(); + expect(typeof capturedRemoteGiveUpHandler).toBe('function'); + }); + + it('sends RPC call for remoteGiveUp when handler is called', async () => { + const keySeed = '0xabcd'; + const relays = ['/dns4/relay.example/tcp/443/wss/p2p/relayPeer']; + + // Capture RPC calls through stream + const outputs: unknown[] = []; + const testStream = await TestDuplexStream.make((message) => { + outputs.push(message); + }); + await testStream.synchronize(); + // eslint-disable-next-line no-new + new PlatformServicesServer( + testStream as unknown as PlatformServicesStream, + makeMockVatWorker, + logger, + ); + await testStream.receiveInput( + makeInitializeRemoteCommsMessageEvent('m1', keySeed, { relays }), + ); + await delay(10); + + // Handler should be captured and callable + expect(capturedRemoteGiveUpHandler).toBeDefined(); + expect(typeof capturedRemoteGiveUpHandler).toBe('function'); + + capturedRemoteGiveUpHandler?.('peer-789'); + await delay(10); + + // Should have sent remoteGiveUp RPC call + const remoteGiveUpCall = outputs.find((outputMessage: unknown) => { + const parsedMessage = outputMessage as { + payload?: { method?: string }; + }; + return parsedMessage.payload?.method === 'remoteGiveUp'; + }); + expect(remoteGiveUpCall).toBeDefined(); + }); + }); + describe('sendRemoteMessage', () => { it('sends message via network layer', async () => { // First initialize remote comms await stream.receiveInput( - makeInitializeRemoteCommsMessageEvent('m0', '0xabcd', [ - '/dns4/relay.example/tcp/443/wss/p2p/relayPeer', - ]), + makeInitializeRemoteCommsMessageEvent('m0', '0xabcd', { + relays: ['/dns4/relay.example/tcp/443/wss/p2p/relayPeer'], + }), ); await delay(10); @@ -431,9 +626,9 @@ describe('PlatformServicesServer', () => { it('stops remote comms and cleans up', async () => { // First initialize await stream.receiveInput( - makeInitializeRemoteCommsMessageEvent('m0', '0xabcd', [ - '/dns4/relay.example/tcp/443/wss/p2p/relayPeer', - ]), + makeInitializeRemoteCommsMessageEvent('m0', '0xabcd', { + relays: ['/dns4/relay.example/tcp/443/wss/p2p/relayPeer'], + }), ); await delay(10); @@ -452,11 +647,11 @@ describe('PlatformServicesServer', () => { it('allows re-initialization after stop', async () => { const keySeed = '0xabcd'; - const knownRelays = ['/dns4/relay.example/tcp/443/wss/p2p/relayPeer']; + const relays = ['/dns4/relay.example/tcp/443/wss/p2p/relayPeer']; // Initialize await stream.receiveInput( - makeInitializeRemoteCommsMessageEvent('m0', keySeed, knownRelays), + makeInitializeRemoteCommsMessageEvent('m0', keySeed, { relays }), ); await delay(10); @@ -469,7 +664,7 @@ describe('PlatformServicesServer', () => { // Re-initialize should work await stream.receiveInput( - makeInitializeRemoteCommsMessageEvent('m2', keySeed, knownRelays), + makeInitializeRemoteCommsMessageEvent('m2', keySeed, { relays }), ); await delay(10); @@ -484,9 +679,9 @@ describe('PlatformServicesServer', () => { it('closes connection via network layer', async () => { // First initialize remote comms await stream.receiveInput( - makeInitializeRemoteCommsMessageEvent('m0', '0xabcd', [ - '/dns4/relay.example/tcp/443/wss/p2p/relayPeer', - ]), + makeInitializeRemoteCommsMessageEvent('m0', '0xabcd', { + relays: ['/dns4/relay.example/tcp/443/wss/p2p/relayPeer'], + }), ); await delay(10); @@ -520,9 +715,9 @@ describe('PlatformServicesServer', () => { it('reconnects peer via network layer', async () => { // First initialize remote comms await stream.receiveInput( - makeInitializeRemoteCommsMessageEvent('m0', '0xabcd', [ - '/dns4/relay.example/tcp/443/wss/p2p/relayPeer', - ]), + makeInitializeRemoteCommsMessageEvent('m0', '0xabcd', { + relays: ['/dns4/relay.example/tcp/443/wss/p2p/relayPeer'], + }), ); await delay(10); @@ -542,9 +737,9 @@ describe('PlatformServicesServer', () => { it('reconnects peer with empty hints', async () => { // First initialize remote comms await stream.receiveInput( - makeInitializeRemoteCommsMessageEvent('m0', '0xabcd', [ - '/dns4/relay.example/tcp/443/wss/p2p/relayPeer', - ]), + makeInitializeRemoteCommsMessageEvent('m0', '0xabcd', { + relays: ['/dns4/relay.example/tcp/443/wss/p2p/relayPeer'], + }), ); await delay(10); diff --git a/packages/kernel-browser-runtime/src/PlatformServicesServer.ts b/packages/kernel-browser-runtime/src/PlatformServicesServer.ts index 40634e250..2f4de62ac 100644 --- a/packages/kernel-browser-runtime/src/PlatformServicesServer.ts +++ b/packages/kernel-browser-runtime/src/PlatformServicesServer.ts @@ -11,6 +11,7 @@ import type { VatConfig, SendRemoteMessage, StopRemoteComms, + RemoteCommsOptions, } from '@metamask/ocap-kernel'; import { initNetwork } from '@metamask/ocap-kernel'; import { @@ -238,13 +239,19 @@ export class PlatformServicesServer { * Initialize network communications. * * @param keySeed - The seed for generating this kernel's secret key. - * @param knownRelays - Array of the peerIDs of relay nodes that can be used to listen for incoming + * @param options - Options for remote communications initialization. + * @param options.relays - Array of the peerIDs of relay nodes that can be used to listen for incoming * connections from other kernels. + * @param options.maxRetryAttempts - Maximum number of reconnection attempts. 0 = infinite (default). + * @param options.maxQueue - Maximum number of messages to queue per peer while reconnecting (default: 200). + * @param _onRemoteGiveUp - Unused parameter (kept for interface compatibility). + * Remote give-up notifications are sent via RPC instead. * @returns A promise that resolves when network access has been initialized. */ async #initializeRemoteComms( keySeed: string, - knownRelays: string[], + options: RemoteCommsOptions, + _onRemoteGiveUp?: (peerId: string) => void, ): Promise { if (this.#sendRemoteMessageFunc) { throw Error('remote comms already initialized'); @@ -252,8 +259,9 @@ export class PlatformServicesServer { const { sendRemoteMessage, stop, closeConnection, reconnectPeer } = await initNetwork( keySeed, - knownRelays, + options, this.#handleRemoteMessage.bind(this), + this.#handleRemoteGiveUp.bind(this), ); this.#sendRemoteMessageFunc = sendRemoteMessage; this.#stopRemoteCommsFunc = stop; @@ -345,5 +353,17 @@ export class PlatformServicesServer { } return ''; } + + /** + * Handle when we give up on a remote connection. + * Notifies the kernel worker via RPC. + * + * @param peerId - The peer ID of the remote we're giving up on. + */ + #handleRemoteGiveUp(peerId: string): void { + this.#rpcClient.call('remoteGiveUp', { peerId }).catch((error) => { + this.#logger.error('Error notifying kernel of remote give up:', error); + }); + } } harden(PlatformServicesServer); diff --git a/packages/kernel-browser-runtime/src/kernel-worker/kernel-worker.ts b/packages/kernel-browser-runtime/src/kernel-worker/kernel-worker.ts index 317511a1b..4fe244572 100644 --- a/packages/kernel-browser-runtime/src/kernel-worker/kernel-worker.ts +++ b/packages/kernel-browser-runtime/src/kernel-worker/kernel-worker.ts @@ -73,5 +73,5 @@ async function main(): Promise { // Initialize remote communications with the relay server passed in the query string const relays = getRelaysFromCurrentLocation(); - await kernel.initRemoteComms(relays); + await kernel.initRemoteComms({ relays }); } diff --git a/packages/kernel-test/src/remote-comms.test.ts b/packages/kernel-test/src/remote-comms.test.ts index ab8f11e6e..8455f99d6 100644 --- a/packages/kernel-test/src/remote-comms.test.ts +++ b/packages/kernel-test/src/remote-comms.test.ts @@ -10,6 +10,7 @@ import type { KRef, PlatformServices, RemoteMessageHandler, + RemoteCommsOptions, } from '@metamask/ocap-kernel'; import { NodejsPlatformServices } from '@ocap/nodejs'; import { describe, it, expect, beforeEach } from 'vitest'; @@ -96,7 +97,7 @@ class DirectNetworkService { async initializeRemoteComms( keySeed: string, - _knownRelays: string[], + _options: RemoteCommsOptions, handler: RemoteMessageHandler, ) { // Generate the actual peer ID from the key seed diff --git a/packages/nodejs/src/kernel/PlatformServices.test.ts b/packages/nodejs/src/kernel/PlatformServices.test.ts index d76a38669..81e43cf23 100644 --- a/packages/nodejs/src/kernel/PlatformServices.test.ts +++ b/packages/nodejs/src/kernel/PlatformServices.test.ts @@ -231,37 +231,74 @@ describe('NodejsPlatformServices', () => { it('initializes remote comms with keySeed and relays', async () => { const service = new NodejsPlatformServices({ workerFilePath }); const keySeed = '0x1234567890abcdef'; - const knownRelays = ['/dns4/relay.example/tcp/443/wss/p2p/relayPeer']; + const relays = ['/dns4/relay.example/tcp/443/wss/p2p/relayPeer']; const remoteHandler = vi.fn(async () => 'response'); - await service.initializeRemoteComms( + await service.initializeRemoteComms(keySeed, { relays }, remoteHandler); + + const { initNetwork } = await import('@metamask/ocap-kernel'); + expect(initNetwork).toHaveBeenCalledWith( keySeed, - knownRelays, - remoteHandler, + { relays }, + expect.any(Function), + undefined, ); + }); + + it('initializes remote comms with all options', async () => { + const service = new NodejsPlatformServices({ workerFilePath }); + const keySeed = '0x1234567890abcdef'; + const options = { + relays: ['/dns4/relay.example/tcp/443/wss/p2p/relayPeer'], + maxRetryAttempts: 5, + maxQueue: 100, + }; + const remoteHandler = vi.fn(async () => 'response'); + + await service.initializeRemoteComms(keySeed, options, remoteHandler); const { initNetwork } = await import('@metamask/ocap-kernel'); expect(initNetwork).toHaveBeenCalledWith( keySeed, - knownRelays, + options, expect.any(Function), + undefined, ); }); - it('throws error if already initialized', async () => { + it('initializes remote comms with onRemoteGiveUp callback', async () => { const service = new NodejsPlatformServices({ workerFilePath }); - const keySeed = '0xabcd'; - const knownRelays = ['/dns4/relay.example/tcp/443/wss/p2p/relayPeer']; + const keySeed = '0x1234567890abcdef'; + const relays = ['/dns4/relay.example/tcp/443/wss/p2p/relayPeer']; const remoteHandler = vi.fn(async () => 'response'); + const giveUpHandler = vi.fn(); await service.initializeRemoteComms( keySeed, - knownRelays, + { relays }, remoteHandler, + giveUpHandler, + ); + + const { initNetwork } = await import('@metamask/ocap-kernel'); + expect(initNetwork).toHaveBeenCalledWith( + keySeed, + { relays }, + expect.any(Function), + giveUpHandler, ); + }); + + it('throws error if already initialized', async () => { + const service = new NodejsPlatformServices({ workerFilePath }); + const keySeed = '0xabcd'; + const relays = ['/dns4/relay.example/tcp/443/wss/p2p/relayPeer']; + const remoteHandler = vi.fn(async () => 'response'); + + await service.initializeRemoteComms(keySeed, { relays }, remoteHandler); await expect( - service.initializeRemoteComms(keySeed, knownRelays, remoteHandler), + service.initializeRemoteComms(keySeed, { relays }, remoteHandler), ).rejects.toThrow('remote comms already initialized'); }); @@ -271,7 +308,7 @@ describe('NodejsPlatformServices', () => { await service.initializeRemoteComms( '0xtest', - ['/dns4/relay.example/tcp/443/wss/p2p/relayPeer'], + { relays: ['/dns4/relay.example/tcp/443/wss/p2p/relayPeer'] }, remoteHandler, ); @@ -279,20 +316,78 @@ describe('NodejsPlatformServices', () => { // This is tested through integration tests expect(service).toBeInstanceOf(NodejsPlatformServices); }); + + it('sends reply message when handler returns non-empty string', async () => { + const service = new NodejsPlatformServices({ workerFilePath }); + const remoteHandler = vi.fn(async () => 'reply-message'); + + await service.initializeRemoteComms('0xtest', {}, remoteHandler); + + // Simulate handleRemoteMessage being called (via initNetwork callback) + // The handler should call sendRemoteMessage if reply is non-empty + mockSendRemoteMessage.mockClear(); + + // Call the handler that was passed to initNetwork + const { initNetwork } = await import('@metamask/ocap-kernel'); + const initNetworkMock = initNetwork as unknown as ReturnType< + typeof vi.fn + >; + const lastCall = + initNetworkMock.mock.calls[initNetworkMock.mock.calls.length - 1]; + const handleRemoteMessage = lastCall?.[2] as ( + from: string, + message: string, + ) => Promise; + expect(handleRemoteMessage).toBeDefined(); + expect(typeof handleRemoteMessage).toBe('function'); + await handleRemoteMessage('peer-123', 'test-message'); + await new Promise((resolve) => setTimeout(resolve, 10)); + expect(mockSendRemoteMessage).toHaveBeenCalledWith( + 'peer-123', + 'reply-message', + [], + ); + }); + + it('does not send reply when handler returns empty string', async () => { + const service = new NodejsPlatformServices({ workerFilePath }); + const remoteHandler = vi.fn(async () => ''); + + await service.initializeRemoteComms('0xtest', {}, remoteHandler); + + mockSendRemoteMessage.mockClear(); + + // Call the handler that was passed to initNetwork + const { initNetwork } = await import('@metamask/ocap-kernel'); + const initNetworkMock = initNetwork as unknown as ReturnType< + typeof vi.fn + >; + const lastCall = + initNetworkMock.mock.calls[initNetworkMock.mock.calls.length - 1]; + const handleRemoteMessage = lastCall?.[2] as ( + from: string, + message: string, + ) => Promise; + + expect(handleRemoteMessage).toBeDefined(); + expect(typeof handleRemoteMessage).toBe('function'); + + await handleRemoteMessage('peer-456', 'test-message'); + await new Promise((resolve) => setTimeout(resolve, 10)); + + // Should not have sent reply + expect(mockSendRemoteMessage).not.toHaveBeenCalled(); + }); }); describe('sendRemoteMessage', () => { it('sends message via network layer', async () => { const service = new NodejsPlatformServices({ workerFilePath }); const keySeed = '0xabcd'; - const knownRelays = ['/dns4/relay.example/tcp/443/wss/p2p/relayPeer']; + const relays = ['/dns4/relay.example/tcp/443/wss/p2p/relayPeer']; const remoteHandler = vi.fn(async () => ''); - await service.initializeRemoteComms( - keySeed, - knownRelays, - remoteHandler, - ); + await service.initializeRemoteComms(keySeed, { relays }, remoteHandler); await service.sendRemoteMessage('peer-456', 'hello', [ '/dns4/relay.example/tcp/443/wss/p2p/relayPeer', @@ -309,7 +404,7 @@ describe('NodejsPlatformServices', () => { const service = new NodejsPlatformServices({ workerFilePath }); await service.initializeRemoteComms( '0xtest', - [], + {}, vi.fn(async () => ''), ); @@ -336,7 +431,7 @@ describe('NodejsPlatformServices', () => { const service = new NodejsPlatformServices({ workerFilePath }); await service.initializeRemoteComms( '0xtest', - [], + {}, vi.fn(async () => ''), ); @@ -354,15 +449,11 @@ describe('NodejsPlatformServices', () => { it('allows re-initialization after stop', async () => { const service = new NodejsPlatformServices({ workerFilePath }); const keySeed = '0xabcd'; - const knownRelays = ['/dns4/relay.example/tcp/443/wss/p2p/relayPeer']; + const relays = ['/dns4/relay.example/tcp/443/wss/p2p/relayPeer']; const remoteHandler = vi.fn(async () => ''); // Initialize - await service.initializeRemoteComms( - keySeed, - knownRelays, - remoteHandler, - ); + await service.initializeRemoteComms(keySeed, { relays }, remoteHandler); const { initNetwork } = await import('@metamask/ocap-kernel'); const initNetworkMock = initNetwork as unknown as ReturnType< @@ -375,11 +466,7 @@ describe('NodejsPlatformServices', () => { expect(mockStop).toHaveBeenCalledOnce(); // Re-initialize should work - await service.initializeRemoteComms( - keySeed, - knownRelays, - remoteHandler, - ); + await service.initializeRemoteComms(keySeed, { relays }, remoteHandler); // Should have called initNetwork again expect(initNetworkMock.mock.calls).toHaveLength(firstCallCount + 1); @@ -389,7 +476,7 @@ describe('NodejsPlatformServices', () => { const service = new NodejsPlatformServices({ workerFilePath }); await service.initializeRemoteComms( '0xtest', - [], + {}, vi.fn(async () => ''), ); @@ -409,7 +496,7 @@ describe('NodejsPlatformServices', () => { const service = new NodejsPlatformServices({ workerFilePath }); await service.initializeRemoteComms( '0xtest', - [], + {}, vi.fn(async () => ''), ); @@ -436,7 +523,7 @@ describe('NodejsPlatformServices', () => { const service = new NodejsPlatformServices({ workerFilePath }); await service.initializeRemoteComms( '0xtest', - [], + {}, vi.fn(async () => ''), ); @@ -459,7 +546,7 @@ describe('NodejsPlatformServices', () => { const service = new NodejsPlatformServices({ workerFilePath }); await service.initializeRemoteComms( '0xtest', - [], + {}, vi.fn(async () => ''), ); @@ -476,7 +563,7 @@ describe('NodejsPlatformServices', () => { const service = new NodejsPlatformServices({ workerFilePath }); await service.initializeRemoteComms( '0xtest', - [], + {}, vi.fn(async () => ''), ); diff --git a/packages/nodejs/src/kernel/PlatformServices.ts b/packages/nodejs/src/kernel/PlatformServices.ts index 265394a4d..169510492 100644 --- a/packages/nodejs/src/kernel/PlatformServices.ts +++ b/packages/nodejs/src/kernel/PlatformServices.ts @@ -8,6 +8,7 @@ import type { RemoteMessageHandler, SendRemoteMessage, StopRemoteComms, + RemoteCommsOptions, } from '@metamask/ocap-kernel'; import { initNetwork } from '@metamask/ocap-kernel'; import { NodeWorkerDuplexStream } from '@metamask/streams'; @@ -219,16 +220,21 @@ export class NodejsPlatformServices implements PlatformServices { * Initialize network communications. * * @param keySeed - The seed for generating this kernel's secret key. - * @param knownRelays - Array of the peerIDs of relay nodes that can be used to listen for incoming + * @param options - Options for remote communications initialization. + * @param options.relays - Array of the peerIDs of relay nodes that can be used to listen for incoming * connections from other kernels. + * @param options.maxRetryAttempts - Maximum number of reconnection attempts. 0 = infinite (default). + * @param options.maxQueue - Maximum number of messages to queue per peer while reconnecting (default: 200). * @param remoteMessageHandler - A handler function to receive remote messages. + * @param onRemoteGiveUp - Optional callback to be called when we give up on a remote. * @returns A promise that resolves once network access has been established * or rejects if there is some problem doing so. */ async initializeRemoteComms( keySeed: string, - knownRelays: string[], + options: RemoteCommsOptions, remoteMessageHandler: (from: string, message: string) => Promise, + onRemoteGiveUp?: (peerId: string) => void, ): Promise { if (this.#sendRemoteMessageFunc) { throw Error('remote comms already initialized'); @@ -237,8 +243,9 @@ export class NodejsPlatformServices implements PlatformServices { const { sendRemoteMessage, stop, closeConnection, reconnectPeer } = await initNetwork( keySeed, - knownRelays, + options, this.#handleRemoteMessage.bind(this), + onRemoteGiveUp, ); this.#sendRemoteMessageFunc = sendRemoteMessage; this.#stopRemoteCommsFunc = stop; diff --git a/packages/nodejs/test/e2e/remote-comms.test.ts b/packages/nodejs/test/e2e/remote-comms.test.ts index 9d0becbbc..903132fb6 100644 --- a/packages/nodejs/test/e2e/remote-comms.test.ts +++ b/packages/nodejs/test/e2e/remote-comms.test.ts @@ -5,6 +5,7 @@ import { makeSQLKernelDatabase } from '@metamask/kernel-store/sqlite/nodejs'; import { Kernel, kunser, makeKernelStore } from '@metamask/ocap-kernel'; import type { KRef } from '@metamask/ocap-kernel'; import { startRelay } from '@ocap/cli/relay'; +import { delay } from '@ocap/repo-tools/test-utils'; import { describe, it, expect, beforeEach, afterEach } from 'vitest'; import { makeTestKernel, runTestVats } from '../helpers/kernel.ts'; @@ -18,7 +19,6 @@ import { restartKernelAndReloadVat, sendRemoteMessage, setupAliceAndBob, - wait, } from '../helpers/remote-comms.ts'; // Increase timeout for network operations @@ -41,7 +41,7 @@ describe.sequential('Remote Communications E2E', () => { // Start the relay server relay = await startRelay(console); // Wait for relay to be fully initialized - await wait(1000); + await delay(1000); // Create two independent kernels with separate storage kernelDatabase1 = await makeSQLKernelDatabase({ @@ -74,15 +74,15 @@ describe.sequential('Remote Communications E2E', () => { if (kernelDatabase2) { kernelDatabase2.close(); } - await wait(1000); + await delay(1000); }); describe('Basic Connectivity', () => { it( 'initializes remote comms on both kernels', async () => { - await kernel1.initRemoteComms(testRelays); - await kernel2.initRemoteComms(testRelays); + await kernel1.initRemoteComms({ relays: testRelays }); + await kernel2.initRemoteComms({ relays: testRelays }); const status1 = await kernel1.getStatus(); const status2 = await kernel2.getStatus(); @@ -157,8 +157,8 @@ describe.sequential('Remote Communications E2E', () => { 'remote relationships should survive kernel restart', async () => { // Initialize remote comms - await kernel1.initRemoteComms(testRelays); - await kernel2.initRemoteComms(testRelays); + await kernel1.initRemoteComms({ relays: testRelays }); + await kernel2.initRemoteComms({ relays: testRelays }); // Launch client vat on kernel1 const clientConfig = makeMaasClientConfig('client1', true); @@ -201,7 +201,7 @@ describe.sequential('Remote Communications E2E', () => { // Kill the server and restart it await serverKernel.stop(); serverKernel = await makeTestKernel(kernelDatabase2, false); - await serverKernel.initRemoteComms(testRelays); + await serverKernel.initRemoteComms({ relays: testRelays }); // Tell the client to talk to the server a second time expectedCount += 1; @@ -217,7 +217,7 @@ describe.sequential('Remote Communications E2E', () => { // Kill the client and restart it await clientKernel.stop(); clientKernel = await makeTestKernel(kernelDatabase1, false); - await clientKernel.initRemoteComms(testRelays); + await clientKernel.initRemoteComms({ relays: testRelays }); // Tell the client to talk to the server a third time expectedCount += 1; @@ -280,7 +280,7 @@ describe.sequential('Remote Communications E2E', () => { it( 'handles connection failure to non-existent peer', async () => { - await kernel1.initRemoteComms(testRelays); + await kernel1.initRemoteComms({ relays: testRelays }); const aliceConfig = makeRemoteVatConfig('Alice'); await launchVatAndGetURL(kernel1, aliceConfig); @@ -386,8 +386,8 @@ describe.sequential('Remote Communications E2E', () => { it( 'queues messages when connection is not established', async () => { - await kernel1.initRemoteComms(testRelays); - await kernel2.initRemoteComms(testRelays); + await kernel1.initRemoteComms({ relays: testRelays }); + await kernel2.initRemoteComms({ relays: testRelays }); const aliceConfig = makeRemoteVatConfig('Alice'); await launchVatAndGetURL(kernel1, aliceConfig); @@ -538,10 +538,10 @@ describe.sequential('Remote Communications E2E', () => { let kernel3: Kernel | undefined; try { - await kernel1.initRemoteComms(testRelays); - await kernel2.initRemoteComms(testRelays); + await kernel1.initRemoteComms({ relays: testRelays }); + await kernel2.initRemoteComms({ relays: testRelays }); kernel3 = await makeTestKernel(kernelDatabase3, true); - await kernel3.initRemoteComms(testRelays); + await kernel3.initRemoteComms({ relays: testRelays }); const aliceConfig = makeRemoteVatConfig('Alice'); const bobConfigInitial = makeRemoteVatConfig('Bob'); @@ -763,7 +763,7 @@ describe.sequential('Remote Communications E2E', () => { // Close connection from kernel1 side await kernel1.closeConnection(peerId2); - await wait(100); + await delay(100); // Try to send a message after closing - should fail const messageAfterClose = kernel1.queueMessage( @@ -781,7 +781,7 @@ describe.sequential('Remote Communications E2E', () => { // Manually reconnect await kernel1.reconnectPeer(peerId2); - await wait(2000); + await delay(2000); // Send message after manual reconnect - should succeed const messageAfterManualReconnect = await sendRemoteMessage( @@ -798,4 +798,102 @@ describe.sequential('Remote Communications E2E', () => { NETWORK_TIMEOUT * 2, ); }); + + describe('Promise Rejection on Remote Give-Up', () => { + it( + 'rejects promises when remote connection is lost after max retries', + async () => { + // Initialize kernel1 with a low maxRetryAttempts to trigger give-up quickly + await kernel1.initRemoteComms({ + relays: testRelays, + maxRetryAttempts: 1, // Only 1 retry attempt before giving up + }); + await kernel2.initRemoteComms({ relays: testRelays }); + + // Set up Alice and Bob manually (can't use setupAliceAndBob as it reinitializes comms) + const aliceConfig = makeRemoteVatConfig('Alice'); + const bobConfig = makeRemoteVatConfig('Bob'); + + await launchVatAndGetURL(kernel1, aliceConfig); + const bobURL = await launchVatAndGetURL(kernel2, bobConfig); + + const aliceRef = getVatRootRef(kernel1, kernelStore1, 'Alice'); + + // Establish connection first by sending a successful message + await sendRemoteMessage(kernel1, aliceRef, bobURL, 'hello', ['Alice']); + + // Now stop kernel2 to trigger connection loss + await kernel2.stop(); + + // Wait for connection loss to be detected and reconnection attempts to fail + await delay(2000); + + // Send a message that will trigger promise creation and eventual rejection + // The message will create a promise with the remote as decider (from URL redemption) + // When we give up on the remote, that promise should be rejected + // The vat should then propagate that rejection to the promise returned here + const messagePromise = kernel1.queueMessage( + aliceRef, + 'sendRemoteMessage', + [bobURL, 'hello', ['Alice']], + ); + + const result = await messagePromise; + const response = kunser(result); + expect(response).toBeInstanceOf(Error); + expect((response as Error).message).toContain( + 'max retries reached or non-retryable error', + ); + }, + NETWORK_TIMEOUT * 2, + ); + + it( + 'resolves promise after reconnection when retries have not been exhausted', + async () => { + const { aliceRef, bobURL } = await setupAliceAndBob( + kernel1, + kernel2, + kernelStore1, + kernelStore2, + testRelays, + ); + + // Send a message that creates a promise with remote as decider + const messagePromise = kernel1.queueMessage( + aliceRef, + 'sendRemoteMessage', + [bobURL, 'hello', ['Alice']], + ); + + // Stop kernel2 before it can respond + await kernel2.stop(); + + // Wait a bit for connection loss to be detected + await delay(500); + + // Restart kernel2 quickly (before max retries, since default is infinite) + // The promise should remain unresolved and resolve normally after reconnection + const bobConfig = makeRemoteVatConfig('Bob'); + // eslint-disable-next-line require-atomic-updates + kernel2 = ( + await restartKernelAndReloadVat( + kernelDatabase2, + false, + testRelays, + bobConfig, + ) + ).kernel; + + // Wait for reconnection + await delay(2000); + + // The message should eventually be delivered and resolved + // The promise was never rejected because retries weren't exhausted + const result = await messagePromise; + expect(kunser(result)).toContain('vat Bob got "hello" from Alice'); + }, + NETWORK_TIMEOUT * 3, + ); + }); }); diff --git a/packages/nodejs/test/helpers/remote-comms.ts b/packages/nodejs/test/helpers/remote-comms.ts index 133f86b62..a5a17050f 100644 --- a/packages/nodejs/test/helpers/remote-comms.ts +++ b/packages/nodejs/test/helpers/remote-comms.ts @@ -134,7 +134,7 @@ export async function restartKernel( relays: string[], ): Promise { const kernel = await makeTestKernel(kernelDatabase, resetStorage); - await kernel.initRemoteComms(relays); + await kernel.initRemoteComms({ relays }); return kernel; } @@ -192,8 +192,8 @@ export async function setupAliceAndBob( peerId1: string; peerId2: string; }> { - await kernel1.initRemoteComms(relays); - await kernel2.initRemoteComms(relays); + await kernel1.initRemoteComms({ relays }); + await kernel2.initRemoteComms({ relays }); const aliceConfig = makeRemoteVatConfig('Alice'); const bobConfig = makeRemoteVatConfig('Bob'); diff --git a/packages/ocap-kernel/src/Kernel.test.ts b/packages/ocap-kernel/src/Kernel.test.ts index a363e3ca7..24ea6c45c 100644 --- a/packages/ocap-kernel/src/Kernel.test.ts +++ b/packages/ocap-kernel/src/Kernel.test.ts @@ -40,7 +40,7 @@ const mocks = vi.hoisted(() => { class RemoteManager { static lastInstance: RemoteManager; - stopRemoteComms = vi.fn().mockResolvedValue(undefined); + cleanup = vi.fn(); isRemoteCommsInitialized = vi.fn().mockReturnValue(false); @@ -725,6 +725,7 @@ describe('Kernel', () => { mockKernelDatabase, ); const queueInstance = mocks.KernelQueue.lastInstance; + const remoteManagerInstance = mocks.RemoteManager.lastInstance; await kernel.launchSubcluster(makeSingleVatClusterConfig()); expect(kernel.getVatIds()).toStrictEqual(['v1']); @@ -742,6 +743,7 @@ describe('Kernel', () => { expect(queueInstance.waitForCrank).toHaveBeenCalledOnce(); expect(endStreamMock).toHaveBeenCalledOnce(); expect(stopRemoteCommsMock).toHaveBeenCalledOnce(); + expect(remoteManagerInstance.cleanup).toHaveBeenCalledOnce(); expect(workerTerminateAllMock).toHaveBeenCalledOnce(); }); diff --git a/packages/ocap-kernel/src/Kernel.ts b/packages/ocap-kernel/src/Kernel.ts index add685405..e881d383b 100644 --- a/packages/ocap-kernel/src/Kernel.ts +++ b/packages/ocap-kernel/src/Kernel.ts @@ -14,6 +14,7 @@ import { KernelServiceManager } from './KernelServiceManager.ts'; import type { KernelService } from './KernelServiceManager.ts'; import { OcapURLManager } from './remotes/OcapURLManager.ts'; import { RemoteManager } from './remotes/RemoteManager.ts'; +import type { RemoteCommsOptions } from './remotes/types.ts'; import { kernelHandlers } from './rpc/index.ts'; import type { PingVatResult } from './rpc/index.ts'; import { makeKernelStore } from './store/index.ts'; @@ -246,11 +247,14 @@ export class Kernel { /** * Initialize the remote comms object. * - * @param relays - The relays to use for the remote comms object. + * @param options - Options for remote communications initialization. + * @param options.relays - The relays to use for the remote comms object. + * @param options.maxRetryAttempts - Maximum number of reconnection attempts. 0 = infinite (default). + * @param options.maxQueue - Maximum number of messages to queue per peer while reconnecting (default: 200). * @returns a promise that resolves when initialization is complete. */ - async initRemoteComms(relays?: string[]): Promise { - await this.#remoteManager.initRemoteComms(relays); + async initRemoteComms(options?: RemoteCommsOptions): Promise { + await this.#remoteManager.initRemoteComms(options); } /** @@ -616,6 +620,7 @@ export class Kernel { await this.#kernelQueue.waitForCrank(); await this.#commandStream.end(); await this.#platformServices.stopRemoteComms(); + this.#remoteManager.cleanup(); await this.#platformServices.terminateAll(); } diff --git a/packages/ocap-kernel/src/index.ts b/packages/ocap-kernel/src/index.ts index 50c80195b..c9fe1413c 100644 --- a/packages/ocap-kernel/src/index.ts +++ b/packages/ocap-kernel/src/index.ts @@ -17,6 +17,7 @@ export type { RemoteMessageHandler, SendRemoteMessage, StopRemoteComms, + RemoteCommsOptions, } from './remotes/types.ts'; export { isVatId, diff --git a/packages/ocap-kernel/src/remotes/ConnectionFactory.test.ts b/packages/ocap-kernel/src/remotes/ConnectionFactory.test.ts index 008d295a7..f20c1ae2f 100644 --- a/packages/ocap-kernel/src/remotes/ConnectionFactory.test.ts +++ b/packages/ocap-kernel/src/remotes/ConnectionFactory.test.ts @@ -195,10 +195,12 @@ describe('ConnectionFactory', () => { * Create a new ConnectionFactory. * * @param signal - The signal to use for the ConnectionFactory. + * @param maxRetryAttempts - Maximum number of retry attempts. * @returns The ConnectionFactory. */ async function createFactory( signal?: AbortSignal, + maxRetryAttempts?: number, ): Promise< Awaited< ReturnType @@ -211,6 +213,7 @@ describe('ConnectionFactory', () => { knownRelays, new Logger(), signal ?? new AbortController().signal, + maxRetryAttempts, ); } @@ -253,6 +256,14 @@ describe('ConnectionFactory', () => { expect(callArgs.peerDiscovery).toBeDefined(); }); + it('accepts maxRetryAttempts parameter', async () => { + const maxRetryAttempts = 5; + factory = await createFactory(undefined, maxRetryAttempts); + + expect(createLibp2p).toHaveBeenCalledOnce(); + expect(libp2pState.startCalled).toBe(true); + }); + it('configures connectionGater to allow all multiaddrs', async () => { factory = await createFactory(); @@ -391,9 +402,18 @@ describe('ConnectionFactory', () => { expect(channel).toBeDefined(); expect(channel.peerId).toBe('peer123'); expect(channel.msgStream).toBeDefined(); + expect(channel.hints).toStrictEqual([]); expect(libp2pState.dials).toHaveLength(1); }); + it('returns channel with hints when provided', async () => { + factory = await createFactory(); + const hints = ['/dns4/hint.example/tcp/443/wss/p2p/hint']; + const channel = await factory.openChannelOnce('peer123', hints); + expect(channel.hints).toStrictEqual(hints); + expect(channel.peerId).toBe('peer123'); + }); + it('tries multiple addresses until one succeeds', async () => { let attemptCount = 0; createLibp2p.mockImplementation(async () => ({ @@ -537,6 +557,44 @@ describe('ConnectionFactory', () => { 'Final connection error', ); }); + + it('throws generic error if all attempts fail without lastError', async () => { + // This shouldn't happen in practice, but test the fallback + createLibp2p.mockImplementation(async () => ({ + start: vi.fn(), + stop: vi.fn(), + peerId: { toString: () => 'test-peer' }, + addEventListener: vi.fn(), + dialProtocol: vi.fn(async () => { + // Throw a non-Error value that gets caught but doesn't set lastError properly + // eslint-disable-next-line @typescript-eslint/only-throw-error + throw null; + }), + handle: vi.fn(), + })); + + factory = await createFactory(); + + await expect(factory.openChannelOnce('peer123')).rejects.toThrow( + 'unable to open channel to peer123', + ); + }); + + it('logs connection attempts', async () => { + factory = await createFactory(); + + await factory.openChannelOnce('peer123'); + + expect(mockLoggerLog).toHaveBeenCalledWith( + expect.stringContaining('contacting peer123 via'), + ); + expect(mockLoggerLog).toHaveBeenCalledWith( + expect.stringContaining('successfully connected to peer123 via'), + ); + expect(mockLoggerLog).toHaveBeenCalledWith( + expect.stringContaining('opened channel to peer123'), + ); + }); }); describe('openChannelWithRetry', () => { @@ -659,6 +717,111 @@ describe('ConnectionFactory', () => { expect(onRetryCallbackCalled).toBe(true); }); + + it('defaults maxRetryAttempts to 0 when not provided', async () => { + let capturedRetryOptions: + | Parameters< + typeof import('@metamask/kernel-utils').retryWithBackoff + >[1] + | undefined; + + // Override the mock retryWithBackoff to capture options + vi.doMock('@metamask/kernel-utils', () => ({ + fromHex: (_hex: string) => new Uint8Array(_hex.length / 2), + retryWithBackoff: async ( + operation: () => Promise, + options?: Parameters< + typeof import('@metamask/kernel-utils').retryWithBackoff + >[1], + ) => { + capturedRetryOptions = options; + return await operation(); + }, + })); + + createLibp2p.mockImplementation(async () => ({ + start: vi.fn(), + stop: vi.fn(), + peerId: { toString: () => 'test-peer' }, + addEventListener: vi.fn(), + dialProtocol: vi.fn(async () => { + return {}; + }), + handle: vi.fn(), + })); + + // Re-import ConnectionFactory to use the new mock + vi.resetModules(); + const { ConnectionFactory } = await import('./ConnectionFactory.ts'); + const { Logger } = await import('@metamask/logger'); + factory = await ConnectionFactory.make( + keySeed, + knownRelays, + new Logger(), + new AbortController().signal, + ); + + await factory.openChannelWithRetry('peer123'); + + // Verify retry was called with maxAttempts defaulting to 0 (infinite) + expect(capturedRetryOptions).toBeDefined(); + expect(capturedRetryOptions).toHaveProperty('maxAttempts', 0); + }); + + it('uses maxRetryAttempts when provided', async () => { + const maxRetryAttempts = 2; + let capturedRetryOptions: + | Parameters< + typeof import('@metamask/kernel-utils').retryWithBackoff + >[1] + | undefined; + + // Override the mock retryWithBackoff to capture options + vi.doMock('@metamask/kernel-utils', () => ({ + fromHex: (_hex: string) => new Uint8Array(_hex.length / 2), + retryWithBackoff: async ( + operation: () => Promise, + options?: Parameters< + typeof import('@metamask/kernel-utils').retryWithBackoff + >[1], + ) => { + capturedRetryOptions = options; + return await operation(); + }, + })); + + createLibp2p.mockImplementation(async () => ({ + start: vi.fn(), + stop: vi.fn(), + peerId: { toString: () => 'test-peer' }, + addEventListener: vi.fn(), + dialProtocol: vi.fn(async () => { + return {}; + }), + handle: vi.fn(), + })); + + // Re-import ConnectionFactory to use the new mock + vi.resetModules(); + const { ConnectionFactory } = await import('./ConnectionFactory.ts'); + const { Logger } = await import('@metamask/logger'); + factory = await ConnectionFactory.make( + keySeed, + knownRelays, + new Logger(), + new AbortController().signal, + maxRetryAttempts, + ); + + await factory.openChannelWithRetry('peer123'); + + // Verify retry was called with maxAttempts + expect(capturedRetryOptions).toBeDefined(); + expect(capturedRetryOptions).toHaveProperty( + 'maxAttempts', + maxRetryAttempts, + ); + }); }); describe('dialIdempotent', () => { diff --git a/packages/ocap-kernel/src/remotes/ConnectionFactory.ts b/packages/ocap-kernel/src/remotes/ConnectionFactory.ts index e2bb459e5..1858f3d3d 100644 --- a/packages/ocap-kernel/src/remotes/ConnectionFactory.ts +++ b/packages/ocap-kernel/src/remotes/ConnectionFactory.ts @@ -36,6 +36,8 @@ export class ConnectionFactory { readonly #keySeed: string; + readonly #maxRetryAttempts: number; + #inboundHandler?: InboundConnectionHandler; /** @@ -45,6 +47,7 @@ export class ConnectionFactory { * @param knownRelays - The known relays to use for the libp2p node. * @param logger - The logger to use for the libp2p node. * @param signal - The signal to use for the libp2p node. + * @param maxRetryAttempts - Maximum number of reconnection attempts. 0 = infinite (default). */ // eslint-disable-next-line no-restricted-syntax private constructor( @@ -52,11 +55,13 @@ export class ConnectionFactory { knownRelays: string[], logger: Logger, signal: AbortSignal, + maxRetryAttempts?: number, ) { this.#keySeed = keySeed; this.#knownRelays = knownRelays; this.#logger = logger; this.#signal = signal; + this.#maxRetryAttempts = maxRetryAttempts ?? 0; } /** @@ -66,6 +71,7 @@ export class ConnectionFactory { * @param knownRelays - The known relays to use for the libp2p node. * @param logger - The logger to use for the libp2p node. * @param signal - The signal to use for the libp2p node. + * @param maxRetryAttempts - Maximum number of reconnection attempts. 0 = infinite (default). * @returns A promise for the new ConnectionFactory instance. */ static async make( @@ -73,8 +79,15 @@ export class ConnectionFactory { knownRelays: string[], logger: Logger, signal: AbortSignal, + maxRetryAttempts?: number, ): Promise { - const factory = new ConnectionFactory(keySeed, knownRelays, logger, signal); + const factory = new ConnectionFactory( + keySeed, + knownRelays, + logger, + signal, + maxRetryAttempts, + ); await factory.#init(); return factory; } @@ -269,7 +282,8 @@ export class ConnectionFactory { peerId: string, hints: string[] = [], ): Promise { - return retryWithBackoff(async () => this.openChannelOnce(peerId, hints), { + const retryOptions: Parameters[1] = { + maxAttempts: this.#maxRetryAttempts, jitter: true, shouldRetry: isRetryableNetworkError, onRetry: ({ attempt, maxAttempts, delayMs }) => { @@ -278,7 +292,11 @@ export class ConnectionFactory { ); }, signal: this.#signal, - }); + }; + return retryWithBackoff( + async () => this.openChannelOnce(peerId, hints), + retryOptions, + ); } /** diff --git a/packages/ocap-kernel/src/remotes/ReconnectionManager.test.ts b/packages/ocap-kernel/src/remotes/ReconnectionManager.test.ts index 8de1837fc..df216eb94 100644 --- a/packages/ocap-kernel/src/remotes/ReconnectionManager.test.ts +++ b/packages/ocap-kernel/src/remotes/ReconnectionManager.test.ts @@ -471,5 +471,47 @@ describe('ReconnectionManager', () => { expect(manager.isReconnecting(peerId)).toBe(true); expect(manager.getAttemptCount(peerId)).toBe(0); }); + + it('resets attempt count when starting new reconnection after max retries exhausted', () => { + const peerId = 'peer1'; + const maxAttempts = 3; + + // Start reconnection and exhaust max attempts + manager.startReconnection(peerId); + manager.incrementAttempt(peerId); + manager.incrementAttempt(peerId); + manager.incrementAttempt(peerId); + + // Max attempts reached + expect(manager.shouldRetry(peerId, maxAttempts)).toBe(false); + expect(manager.getAttemptCount(peerId)).toBe(3); + + // Stop reconnection (simulating giving up) + manager.stopReconnection(peerId); + expect(manager.isReconnecting(peerId)).toBe(false); + // Attempt count is still 3 (not reset by stopReconnection) + + // Start a new reconnection session - should reset attempt count + manager.startReconnection(peerId); + expect(manager.isReconnecting(peerId)).toBe(true); + expect(manager.getAttemptCount(peerId)).toBe(0); + // Should now allow retries again + expect(manager.shouldRetry(peerId, maxAttempts)).toBe(true); + }); + + it('does not reset attempt count when already reconnecting', () => { + const peerId = 'peer1'; + + // Start reconnection and make some attempts + manager.startReconnection(peerId); + manager.incrementAttempt(peerId); + manager.incrementAttempt(peerId); + expect(manager.getAttemptCount(peerId)).toBe(2); + + // Calling startReconnection again (idempotent) should not reset + manager.startReconnection(peerId); + expect(manager.getAttemptCount(peerId)).toBe(2); + expect(manager.isReconnecting(peerId)).toBe(true); + }); }); }); diff --git a/packages/ocap-kernel/src/remotes/ReconnectionManager.ts b/packages/ocap-kernel/src/remotes/ReconnectionManager.ts index 9c19c62c0..3664a2472 100644 --- a/packages/ocap-kernel/src/remotes/ReconnectionManager.ts +++ b/packages/ocap-kernel/src/remotes/ReconnectionManager.ts @@ -29,11 +29,17 @@ export class ReconnectionManager { /** * Start reconnection for a peer. + * Resets attempt count when starting a new reconnection session. * * @param peerId - The peer ID to start reconnection for. */ startReconnection(peerId: string): void { const state = this.#getState(peerId); + // Reset attempt count when starting a new reconnection session + // This allows retries after max attempts were previously exhausted + if (!state.isReconnecting) { + state.attemptCount = 0; + } state.isReconnecting = true; } diff --git a/packages/ocap-kernel/src/remotes/RemoteHandle.test.ts b/packages/ocap-kernel/src/remotes/RemoteHandle.test.ts index 0c998855f..255e8fcd9 100644 --- a/packages/ocap-kernel/src/remotes/RemoteHandle.test.ts +++ b/packages/ocap-kernel/src/remotes/RemoteHandle.test.ts @@ -479,4 +479,153 @@ describe('RemoteHandle', () => { 'unknown remote message type bogus', ); }); + + it('rejectPendingRedemptions rejects all pending redemptions', async () => { + const remote = makeRemote(); + const errorMessage = 'Connection lost'; + + // Start multiple URL redemptions + const promise1 = remote.redeemOcapURL('url1'); + const promise2 = remote.redeemOcapURL('url2'); + const promise3 = remote.redeemOcapURL('url3'); + + // Reject all pending redemptions + remote.rejectPendingRedemptions(errorMessage); + + // All promises should be rejected with the error + await expect(promise1).rejects.toThrow(errorMessage); + await expect(promise2).rejects.toThrow(errorMessage); + await expect(promise3).rejects.toThrow(errorMessage); + }); + + it('rejectPendingRedemptions clears pending redemptions map', async () => { + const remote = makeRemote(); + const errorMessage = 'Connection lost'; + + // Start a URL redemption + const promise = remote.redeemOcapURL('url1'); + + // Reject all pending redemptions + remote.rejectPendingRedemptions(errorMessage); + + // Try to handle a reply for the rejected redemption - should fail + const redeemURLReply = { + method: 'redeemURLReply', + params: [true, '1', 'ro+1'], + }; + await expect( + remote.handleRemoteMessage(JSON.stringify(redeemURLReply)), + ).rejects.toThrow('unknown URL redemption reply key 1'); + + await expect(promise).rejects.toThrow(errorMessage); + }); + + it('rejectPendingRedemptions handles empty pending redemptions', () => { + const remote = makeRemote(); + const errorMessage = 'Connection lost'; + + // Should not throw when there are no pending redemptions + expect(() => remote.rejectPendingRedemptions(errorMessage)).not.toThrow(); + }); + + it('redeemOcapURL increments redemption counter for multiple redemptions', async () => { + const remote = makeRemote(); + const mockOcapURL1 = 'url1'; + const mockOcapURL2 = 'url2'; + const mockOcapURL3 = 'url3'; + + // Start multiple redemptions + const promise1 = remote.redeemOcapURL(mockOcapURL1); + const promise2 = remote.redeemOcapURL(mockOcapURL2); + const promise3 = remote.redeemOcapURL(mockOcapURL3); + + // Verify each redemption uses a different reply key + expect(mockRemoteComms.sendRemoteMessage).toHaveBeenCalledWith( + mockRemotePeerId, + JSON.stringify({ + method: 'redeemURL', + params: [mockOcapURL1, '1'], + }), + undefined, + ); + expect(mockRemoteComms.sendRemoteMessage).toHaveBeenCalledWith( + mockRemotePeerId, + JSON.stringify({ + method: 'redeemURL', + params: [mockOcapURL2, '2'], + }), + undefined, + ); + expect(mockRemoteComms.sendRemoteMessage).toHaveBeenCalledWith( + mockRemotePeerId, + JSON.stringify({ + method: 'redeemURL', + params: [mockOcapURL3, '3'], + }), + undefined, + ); + + // Resolve all redemptions + await remote.handleRemoteMessage( + JSON.stringify({ + method: 'redeemURLReply', + params: [true, '1', 'ro+1'], + }), + ); + await remote.handleRemoteMessage( + JSON.stringify({ + method: 'redeemURLReply', + params: [true, '2', 'ro+2'], + }), + ); + await remote.handleRemoteMessage( + JSON.stringify({ + method: 'redeemURLReply', + params: [true, '3', 'ro+3'], + }), + ); + + await promise1; + await promise2; + await promise3; + }); + + it('handles multiple concurrent URL redemptions independently', async () => { + const remote = makeRemote(); + const mockOcapURL1 = 'url1'; + const mockOcapURL2 = 'url2'; + const mockURLResolutionRRef1 = 'ro+1'; + const mockURLResolutionRRef2 = 'ro+2'; + + // Start two concurrent redemptions + const promise1 = remote.redeemOcapURL(mockOcapURL1); + const promise2 = remote.redeemOcapURL(mockOcapURL2); + + // Resolve them in reverse order to verify they're handled independently + await remote.handleRemoteMessage( + JSON.stringify({ + method: 'redeemURLReply', + params: [true, '2', mockURLResolutionRRef2], + }), + ); + await remote.handleRemoteMessage( + JSON.stringify({ + method: 'redeemURLReply', + params: [true, '1', mockURLResolutionRRef1], + }), + ); + + const kref1 = await promise1; + const kref2 = await promise2; + + // Verify each promise resolved with the correct value based on its reply key + expect(kref1).toBe( + mockKernelStore.translateRefEtoK(remote.remoteId, mockURLResolutionRRef1), + ); + expect(kref2).toBe( + mockKernelStore.translateRefEtoK(remote.remoteId, mockURLResolutionRRef2), + ); + // Verify they resolved independently (different values) + expect(kref1).not.toBe(kref2); + }); }); diff --git a/packages/ocap-kernel/src/remotes/RemoteHandle.ts b/packages/ocap-kernel/src/remotes/RemoteHandle.ts index 61c460d2f..e764dd863 100644 --- a/packages/ocap-kernel/src/remotes/RemoteHandle.ts +++ b/packages/ocap-kernel/src/remotes/RemoteHandle.ts @@ -66,10 +66,6 @@ export class RemoteHandle implements EndpointHandle { /** The peer ID of the remote kernel this is connected to. */ readonly #peerId: string; - /** Logger for outputting messages (such as errors) to the console */ - // eslint-disable-next-line no-unused-private-class-members - readonly #logger: Logger; - /** Storage holding the kernel's persistent state. */ readonly #kernelStore: KernelStore; @@ -85,7 +81,7 @@ export class RemoteHandle implements EndpointHandle { /** Pending URL redemption requests that have not yet been responded to. */ readonly #pendingRedemptions: Map< string, - [(ref: string) => void, (problem: string) => void] + [(ref: string) => void, (problem: string | Error) => void] > = new Map(); /** Generation counter for keys to match URL redemption replies to requests. */ @@ -104,7 +100,6 @@ export class RemoteHandle implements EndpointHandle { * @param params.kernelQueue - The kernel's queue. * @param params.remoteComms - Remote comms object to access the network. * @param params.locationHints - Possible contact points to reach the other end. - * @param params.logger - Optional logger for error and diagnostic output. */ // eslint-disable-next-line no-restricted-syntax private constructor({ @@ -114,14 +109,9 @@ export class RemoteHandle implements EndpointHandle { kernelQueue, remoteComms, locationHints, - logger, }: RemoteHandleConstructorProps) { this.remoteId = remoteId; this.#peerId = peerId; - const loggerTags = { tags: ['remote', `${remoteId}`] }; - this.#logger = logger - ? logger.subLogger(loggerTags) - : new Logger(loggerTags); this.#kernelStore = kernelStore; this.#kernelQueue = kernelQueue; this.#remoteComms = remoteComms; @@ -461,4 +451,18 @@ export class RemoteHandle implements EndpointHandle { }); return promise; } + + /** + * Reject all pending URL redemptions with the given error message. + * Called when we give up on this remote connection. + * + * @param errorMessage - The error message to reject with. + */ + rejectPendingRedemptions(errorMessage: string): void { + const error = Error(errorMessage); + for (const [, [, reject]] of this.#pendingRedemptions) { + reject(error); + } + this.#pendingRedemptions.clear(); + } } diff --git a/packages/ocap-kernel/src/remotes/RemoteManager.test.ts b/packages/ocap-kernel/src/remotes/RemoteManager.test.ts index d9ea14a45..bb9d2783c 100644 --- a/packages/ocap-kernel/src/remotes/RemoteManager.test.ts +++ b/packages/ocap-kernel/src/remotes/RemoteManager.test.ts @@ -70,15 +70,69 @@ describe('RemoteManager', () => { vi.mocked(remoteComms.initRemoteComms).mockResolvedValue(mockRemoteComms); remoteManager.setMessageHandler(messageHandler); - await remoteManager.initRemoteComms(['relay1', 'relay2']); + await remoteManager.initRemoteComms({ relays: ['relay1', 'relay2'] }); expect(remoteComms.initRemoteComms).toHaveBeenCalledWith( kernelStore, mockPlatformServices, messageHandler, - ['relay1', 'relay2'], + { relays: ['relay1', 'relay2'] }, + logger, + undefined, + expect.any(Function), + ); + }); + + it('initializes remote comms with all options', async () => { + const messageHandler = vi.fn(); + vi.mocked(remoteComms.initRemoteComms).mockResolvedValue(mockRemoteComms); + + remoteManager.setMessageHandler(messageHandler); + await remoteManager.initRemoteComms({ + relays: ['relay1', 'relay2'], + maxRetryAttempts: 5, + maxQueue: 100, + }); + + expect(remoteComms.initRemoteComms).toHaveBeenCalledWith( + kernelStore, + mockPlatformServices, + messageHandler, + { + relays: ['relay1', 'relay2'], + maxRetryAttempts: 5, + maxQueue: 100, + }, logger, undefined, + expect.any(Function), + ); + }); + + it('passes keySeed to initRemoteComms', async () => { + const keySeed = '0x1234567890abcdef'; + const managerWithKeySeed = new RemoteManager({ + platformServices: mockPlatformServices, + kernelStore, + kernelQueue: mockKernelQueue, + logger, + keySeed, + }); + + const messageHandler = vi.fn(); + vi.mocked(remoteComms.initRemoteComms).mockResolvedValue(mockRemoteComms); + + managerWithKeySeed.setMessageHandler(messageHandler); + await managerWithKeySeed.initRemoteComms(); + + expect(remoteComms.initRemoteComms).toHaveBeenCalledWith( + kernelStore, + mockPlatformServices, + messageHandler, + {}, + logger, + keySeed, + expect.any(Function), ); }); @@ -107,7 +161,8 @@ describe('RemoteManager', () => { const remote2Id = remote2.remoteId; // Stop remote comms (simulating shutdown) - await remoteManager.stopRemoteComms(); + await mockPlatformServices.stopRemoteComms(); + remoteManager.cleanup(); // Create a new RemoteManager instance (simulating restart) const newRemoteManager = new RemoteManager({ @@ -184,20 +239,32 @@ describe('RemoteManager', () => { it('closes connection to peer', async () => { await remoteManager.closeConnection('peer123'); - expect(mockRemoteComms.closeConnection).toHaveBeenCalledWith('peer123'); + expect(mockPlatformServices.closeConnection).toHaveBeenCalledWith( + 'peer123', + ); + }); + + it('closes connection to peer that does not exist', async () => { + await remoteManager.closeConnection('non-existent-peer'); + expect(mockPlatformServices.closeConnection).toHaveBeenCalledWith( + 'non-existent-peer', + ); }); it('reconnects peer with hints', async () => { await remoteManager.reconnectPeer('peer123', ['relay1', 'relay2']); - expect(mockRemoteComms.reconnectPeer).toHaveBeenCalledWith('peer123', [ - 'relay1', - 'relay2', - ]); + expect(mockPlatformServices.reconnectPeer).toHaveBeenCalledWith( + 'peer123', + ['relay1', 'relay2'], + ); }); it('reconnects peer with empty hints when hints not provided', async () => { await remoteManager.reconnectPeer('peer123'); - expect(mockRemoteComms.reconnectPeer).toHaveBeenCalledWith('peer123', []); + expect(mockPlatformServices.reconnectPeer).toHaveBeenCalledWith( + 'peer123', + [], + ); }); it('gets remote comms after initialization', () => { @@ -319,7 +386,8 @@ describe('RemoteManager', () => { const { remoteId } = remote; // Stop and restart - await remoteManager.stopRemoteComms(); + await mockPlatformServices.stopRemoteComms(); + remoteManager.cleanup(); const messageHandler = vi.fn(); vi.mocked(remoteComms.initRemoteComms).mockResolvedValue(mockRemoteComms); @@ -352,7 +420,7 @@ describe('RemoteManager', () => { }); }); - describe('stopRemoteComms', () => { + describe('cleanup', () => { beforeEach(async () => { const messageHandler = vi.fn(); vi.mocked(remoteComms.initRemoteComms).mockResolvedValue(mockRemoteComms); @@ -360,19 +428,13 @@ describe('RemoteManager', () => { await remoteManager.initRemoteComms(); }); - it('stops remote comms and calls stopRemoteComms', async () => { - await remoteManager.stopRemoteComms(); - - expect(mockRemoteComms.stopRemoteComms).toHaveBeenCalledOnce(); - }); - - it('clears remoteComms after stopping', async () => { - await remoteManager.stopRemoteComms(); + it('clears remoteComms after cleanup', () => { + remoteManager.cleanup(); expect(remoteManager.isRemoteCommsInitialized()).toBe(false); }); - it('clears all remote handles after stopping', async () => { + it('clears all remote handles after cleanup', () => { // Establish some remotes remoteManager.establishRemote('peer1'); remoteManager.establishRemote('peer2'); @@ -383,45 +445,45 @@ describe('RemoteManager', () => { expect(remoteManager.remoteFor('peer2')).toBeDefined(); expect(remoteManager.remoteFor('peer3')).toBeDefined(); - await remoteManager.stopRemoteComms(); + remoteManager.cleanup(); - // After stop, trying to get remotes should throw or create new ones + // After cleanup, trying to get remotes should throw or create new ones expect(remoteManager.isRemoteCommsInitialized()).toBe(false); }); - it('throws when calling getPeerId after stop', async () => { - await remoteManager.stopRemoteComms(); + it('throws when calling getPeerId after cleanup', () => { + remoteManager.cleanup(); expect(() => remoteManager.getPeerId()).toThrow( 'Remote comms not initialized', ); }); - it('throws when calling sendRemoteMessage after stop', async () => { - await remoteManager.stopRemoteComms(); + it('throws when calling sendRemoteMessage after cleanup', async () => { + remoteManager.cleanup(); await expect( remoteManager.sendRemoteMessage('peer1', 'test'), ).rejects.toThrow('Remote comms not initialized'); }); - it('throws when calling closeConnection after stop', async () => { - await remoteManager.stopRemoteComms(); + it('throws when calling closeConnection after cleanup', async () => { + remoteManager.cleanup(); await expect(remoteManager.closeConnection('peer1')).rejects.toThrow( 'Remote comms not initialized', ); }); - it('throws when calling reconnectPeer after stop', async () => { - await remoteManager.stopRemoteComms(); + it('throws when calling reconnectPeer after cleanup', async () => { + remoteManager.cleanup(); await expect(remoteManager.reconnectPeer('peer1')).rejects.toThrow( 'Remote comms not initialized', ); }); - it('can be called when remote comms is not initialized', async () => { + it('can be called when remote comms is not initialized', () => { const newManager = new RemoteManager({ platformServices: mockPlatformServices, kernelStore, @@ -430,24 +492,23 @@ describe('RemoteManager', () => { }); // Should not throw - const result = await newManager.stopRemoteComms(); - expect(result).toBeUndefined(); + expect(() => newManager.cleanup()).not.toThrow(); }); - it('allows re-initialization after stop', async () => { - await remoteManager.stopRemoteComms(); + it('allows re-initialization after cleanup', async () => { + remoteManager.cleanup(); // Should be able to initialize again const messageHandler = vi.fn(); vi.mocked(remoteComms.initRemoteComms).mockResolvedValue(mockRemoteComms); remoteManager.setMessageHandler(messageHandler); - await remoteManager.initRemoteComms(['relay1']); + await remoteManager.initRemoteComms({ relays: ['relay1'] }); expect(remoteManager.isRemoteCommsInitialized()).toBe(true); expect(remoteComms.initRemoteComms).toHaveBeenCalledTimes(2); }); - it('clears remote handles by peer ID', async () => { + it('clears remote handles by peer ID', () => { const remote1 = remoteManager.establishRemote('peer1'); const remote2 = remoteManager.establishRemote('peer2'); @@ -455,10 +516,61 @@ describe('RemoteManager', () => { expect(remoteManager.remoteFor('peer1')).toBe(remote1); expect(remoteManager.remoteFor('peer2')).toBe(remote2); - await remoteManager.stopRemoteComms(); + remoteManager.cleanup(); - // After stop, remotes are cleared + // After cleanup, remotes are cleared expect(remoteManager.isRemoteCommsInitialized()).toBe(false); }); }); + + describe('handleRemoteGiveUp', () => { + beforeEach(async () => { + const messageHandler = vi.fn(); + vi.mocked(remoteComms.initRemoteComms).mockResolvedValue(mockRemoteComms); + remoteManager.setMessageHandler(messageHandler); + await remoteManager.initRemoteComms(); + }); + + it('handles remote give up callback when remote exists', () => { + const peerId = 'peer-to-give-up'; + const remote = remoteManager.establishRemote(peerId); + const rejectPendingRedemptionsSpy = vi.spyOn( + remote, + 'rejectPendingRedemptions', + ); + // Get the callback that was passed to initRemoteComms + const initCall = vi.mocked(remoteComms.initRemoteComms).mock.calls[0]; + const onRemoteGiveUp = initCall?.[6] as (peerId: string) => void; + onRemoteGiveUp(peerId); + // Verify pending redemptions were rejected + expect(rejectPendingRedemptionsSpy).toHaveBeenCalledWith( + `Remote connection lost: ${peerId} (max retries reached or non-retryable error)`, + ); + }); + + it('handles remote give up callback when remote does not exist', () => { + const peerId = 'non-existent-peer'; + const initCall = vi.mocked(remoteComms.initRemoteComms).mock.calls[0]; + const onRemoteGiveUp = initCall?.[6] as (peerId: string) => void; + expect(() => onRemoteGiveUp(peerId)).not.toThrow(); + }); + + it('handles remote give up and processes promises when they exist', () => { + const peerId = 'peer-with-promises'; + remoteManager.establishRemote(peerId); + const initCall = vi.mocked(remoteComms.initRemoteComms).mock.calls[0]; + const onRemoteGiveUp = initCall?.[6] as (peerId: string) => void; + expect(() => onRemoteGiveUp(peerId)).not.toThrow(); + }); + + it('handles remote give up with no promises', () => { + const peerId = 'peer-with-no-promises'; + remoteManager.establishRemote(peerId); + const resolvePromisesSpy = vi.spyOn(mockKernelQueue, 'resolvePromises'); + const initCall = vi.mocked(remoteComms.initRemoteComms).mock.calls[0]; + const onRemoteGiveUp = initCall?.[6] as (peerId: string) => void; + onRemoteGiveUp(peerId); + expect(resolvePromisesSpy).not.toHaveBeenCalled(); + }); + }); }); diff --git a/packages/ocap-kernel/src/remotes/RemoteManager.ts b/packages/ocap-kernel/src/remotes/RemoteManager.ts index cada6e701..a71384e7c 100644 --- a/packages/ocap-kernel/src/remotes/RemoteManager.ts +++ b/packages/ocap-kernel/src/remotes/RemoteManager.ts @@ -1,11 +1,17 @@ import type { Logger } from '@metamask/logger'; import type { KernelQueue } from '../KernelQueue.ts'; -import type { KernelStore } from '../store/index.ts'; -import type { PlatformServices, RemoteId } from '../types.ts'; import { initRemoteComms } from './remote-comms.ts'; import { RemoteHandle } from './RemoteHandle.ts'; -import type { RemoteComms, RemoteMessageHandler, RemoteInfo } from './types.ts'; +import { kser } from '../liveslots/kernel-marshal.ts'; +import type { PlatformServices, RemoteId } from '../types.ts'; +import type { + RemoteComms, + RemoteMessageHandler, + RemoteInfo, + RemoteCommsOptions, +} from './types.ts'; +import type { KernelStore } from '../store/index.ts'; type RemoteManagerConstructorProps = { platformServices: PlatformServices; @@ -70,13 +76,46 @@ export class RemoteManager { this.#messageHandler = handler; } + /** + * Handle when we give up on a remote (after max retries or non-retryable error). + * Rejects all promises for which this remote is the decider. + * + * @param peerId - The peer ID of the remote we're giving up on. + */ + #handleRemoteGiveUp(peerId: string): void { + // Find the RemoteId for this peerId + const remote = this.#remotesByPeer.get(peerId); + if (!remote) { + // Remote not found - might have been cleaned up already + return; + } + + const { remoteId } = remote; + const failure = kser( + Error( + `Remote connection lost: ${peerId} (max retries reached or non-retryable error)`, + ), + ); + + // Reject pending URL redemptions in the RemoteHandle + // These are JavaScript promises that will propagate rejection to kernel promises + remote.rejectPendingRedemptions( + `Remote connection lost: ${peerId} (max retries reached or non-retryable error)`, + ); + + // Reject all promises for which this remote is the decider + for (const kpid of this.#kernelStore.getPromisesByDecider(remoteId)) { + this.#kernelQueue.resolvePromises(remoteId, [[kpid, true, failure]]); + } + } + /** * Initialize the remote comms object at kernel startup. * - * @param relays - The relays to use for the remote comms object. + * @param options - Options for remote communications initialization. * @returns a promise that resolves when initialization is complete. */ - async initRemoteComms(relays?: string[]): Promise { + async initRemoteComms(options?: RemoteCommsOptions): Promise { if (!this.#messageHandler) { throw Error( 'Message handler must be set before initializing remote comms', @@ -87,9 +126,10 @@ export class RemoteManager { this.#kernelStore, this.#platformServices, this.#messageHandler, - relays, + options ?? {}, this.#logger, this.#keySeed, + this.#handleRemoteGiveUp.bind(this), ); // Restore all remotes that were previously established @@ -102,12 +142,10 @@ export class RemoteManager { } /** - * Stop the remote comms object. - * - * @returns a promise that resolves when stopping is complete. + * Clean up remote manager state. + * This should be called when remote comms are stopped externally. */ - async stopRemoteComms(): Promise { - await this.#remoteComms?.stopRemoteComms(); + cleanup(): void { this.#remoteComms = undefined; this.#remotes.clear(); this.#remotesByPeer.clear(); @@ -243,7 +281,8 @@ export class RemoteManager { * @param peerId - The peer ID to close the connection for. */ async closeConnection(peerId: string): Promise { - await this.getRemoteComms().closeConnection(peerId); + this.getRemoteComms(); // Ensure remote comms is initialized + await this.#platformServices.closeConnection(peerId); } /** @@ -254,6 +293,7 @@ export class RemoteManager { * @param hints - Optional hints for reconnection. */ async reconnectPeer(peerId: string, hints: string[] = []): Promise { - await this.getRemoteComms().reconnectPeer(peerId, hints); + this.getRemoteComms(); // Ensure remote comms is initialized + await this.#platformServices.reconnectPeer(peerId, hints); } } diff --git a/packages/ocap-kernel/src/remotes/network.test.ts b/packages/ocap-kernel/src/remotes/network.test.ts index 1c64fe7fa..270de38b3 100644 --- a/packages/ocap-kernel/src/remotes/network.test.ts +++ b/packages/ocap-kernel/src/remotes/network.test.ts @@ -234,18 +234,54 @@ describe('network.initNetwork', () => { '/dns4/relay2.example/tcp/443/wss/p2p/relay2', ]; - await initNetwork(keySeed, knownRelays, vi.fn()); + await initNetwork(keySeed, { relays: knownRelays }, vi.fn()); expect(ConnectionFactory.make).toHaveBeenCalledWith( keySeed, knownRelays, expect.any(Object), // Logger instance expect.any(AbortSignal), // signal from AbortController + undefined, // maxRetryAttempts (optional) ); }); + it('passes maxRetryAttempts to ConnectionFactory.make', async () => { + const { ConnectionFactory } = await import('./ConnectionFactory.ts'); + const keySeed = '0xabcd'; + const maxRetryAttempts = 5; + + await initNetwork(keySeed, { relays: [], maxRetryAttempts }, vi.fn()); + + expect(ConnectionFactory.make).toHaveBeenCalledWith( + keySeed, + [], + expect.any(Object), + expect.any(AbortSignal), + maxRetryAttempts, + ); + }); + + it('uses maxQueue option for MessageQueue', async () => { + const maxQueue = 100; + + const mockChannel = createMockChannel('peer-1'); + mockConnectionFactory.dialIdempotent.mockResolvedValue(mockChannel); + mockReconnectionManager.isReconnecting.mockReturnValue(true); + + const { sendRemoteMessage } = await initNetwork( + '0x1234', + { maxQueue }, + vi.fn(), + ); + + await sendRemoteMessage('peer-1', 'msg'); + + // Verify message was queued (MessageQueue is created lazily with maxQueue) + expect(mockMessageQueue.enqueue).toHaveBeenCalledWith('msg', []); + }); + it('returns sendRemoteMessage, stop, closeConnection, and reconnectPeer', async () => { - const result = await initNetwork('0x1234', [], vi.fn()); + const result = await initNetwork('0x1234', {}, vi.fn()); expect(result).toHaveProperty('sendRemoteMessage'); expect(result).toHaveProperty('stop'); @@ -265,7 +301,9 @@ describe('network.initNetwork', () => { const { sendRemoteMessage } = await initNetwork( '0x1234', - ['/dns4/relay.example/tcp/443/wss/p2p/relay1'], + { + relays: ['/dns4/relay.example/tcp/443/wss/p2p/relay1'], + }, vi.fn(), ); @@ -285,7 +323,7 @@ describe('network.initNetwork', () => { const mockChannel = createMockChannel('peer-1'); mockConnectionFactory.dialIdempotent.mockResolvedValue(mockChannel); - const { sendRemoteMessage } = await initNetwork('0x1234', [], vi.fn()); + const { sendRemoteMessage } = await initNetwork('0x1234', {}, vi.fn()); await sendRemoteMessage('peer-1', 'msg1'); await sendRemoteMessage('peer-1', 'msg2'); @@ -301,7 +339,7 @@ describe('network.initNetwork', () => { .mockResolvedValueOnce(mockChannel1) .mockResolvedValueOnce(mockChannel2); - const { sendRemoteMessage } = await initNetwork('0x1234', [], vi.fn()); + const { sendRemoteMessage } = await initNetwork('0x1234', {}, vi.fn()); await sendRemoteMessage('peer-1', 'hello'); await sendRemoteMessage('peer-2', 'world'); @@ -314,7 +352,7 @@ describe('network.initNetwork', () => { mockConnectionFactory.dialIdempotent.mockResolvedValue(mockChannel); const hints = ['/dns4/hint.example/tcp/443/wss/p2p/hint']; - const { sendRemoteMessage } = await initNetwork('0x1234', [], vi.fn()); + const { sendRemoteMessage } = await initNetwork('0x1234', {}, vi.fn()); await sendRemoteMessage('peer-1', 'hello', hints); @@ -328,7 +366,7 @@ describe('network.initNetwork', () => { describe('inbound connections', () => { it('registers inbound connection handler', async () => { - await initNetwork('0x1234', [], vi.fn()); + await initNetwork('0x1234', {}, vi.fn()); expect(mockConnectionFactory.onInboundConnection).toHaveBeenCalledWith( expect.any(Function), @@ -345,7 +383,7 @@ describe('network.initNetwork', () => { }, ); - await initNetwork('0x1234', [], remoteHandler); + await initNetwork('0x1234', {}, remoteHandler); const mockChannel = createMockChannel('inbound-peer'); const messageBuffer = new TextEncoder().encode('test-message'); @@ -380,7 +418,7 @@ describe('network.initNetwork', () => { }, ); - await initNetwork('0x1234', [], remoteHandler); + await initNetwork('0x1234', {}, remoteHandler); const mockChannel = createMockChannel('inbound-peer'); const messageBuffer = new TextEncoder().encode('test-message'); @@ -413,7 +451,7 @@ describe('network.initNetwork', () => { mockMessageQueue.length = 1; mockReconnectionManager.isReconnecting.mockReturnValue(true); - const { sendRemoteMessage } = await initNetwork('0x1234', [], vi.fn()); + const { sendRemoteMessage } = await initNetwork('0x1234', {}, vi.fn()); await sendRemoteMessage('peer-1', 'queued-msg'); @@ -428,7 +466,7 @@ describe('network.initNetwork', () => { ); mockConnectionFactory.dialIdempotent.mockResolvedValue(mockChannel); - const { sendRemoteMessage } = await initNetwork('0x1234', [], vi.fn()); + const { sendRemoteMessage } = await initNetwork('0x1234', {}, vi.fn()); await sendRemoteMessage('peer-1', 'msg1'); @@ -451,7 +489,7 @@ describe('network.initNetwork', () => { }, ); - await initNetwork('0x1234', [], vi.fn()); + await initNetwork('0x1234', {}, vi.fn()); const mockChannel = createMockChannel('peer-1'); mockChannel.msgStream.read.mockRejectedValue(new Error('Read failed')); @@ -473,7 +511,7 @@ describe('network.initNetwork', () => { }, ); - await initNetwork('0x1234', [], vi.fn()); + await initNetwork('0x1234', {}, vi.fn()); const mockChannel = createMockChannel('peer-1'); const gracefulDisconnectError = Object.assign(new Error('SCTP failure'), { @@ -505,7 +543,7 @@ describe('network.initNetwork', () => { }, ); - const { stop } = await initNetwork('0x1234', [], vi.fn()); + const { stop } = await initNetwork('0x1234', {}, vi.fn()); const mockChannel = createMockChannel('peer-1'); // Make read resolve after stop so loop continues and checks signal.aborted @@ -549,7 +587,7 @@ describe('network.initNetwork', () => { ); const remoteHandler = vi.fn().mockResolvedValue('ok'); - await initNetwork('0x1234', [], remoteHandler); + await initNetwork('0x1234', {}, remoteHandler); const mockChannel = createMockChannel('peer-1'); // First read returns undefined, which means stream ended - loop should break @@ -589,7 +627,7 @@ describe('network.initNetwork', () => { .mockResolvedValueOnce(mockChannel) // Initial connection .mockResolvedValueOnce(mockChannel); // Reconnection succeeds - const { sendRemoteMessage } = await initNetwork('0x1234', [], vi.fn()); + const { sendRemoteMessage } = await initNetwork('0x1234', {}, vi.fn()); // First send establishes channel await sendRemoteMessage('peer-1', 'initial-msg'); @@ -606,17 +644,75 @@ describe('network.initNetwork', () => { expect(mockChannel.msgStream.write).toHaveBeenCalledTimes(3); }); }); + + it('resets backoff once after successful flush completion', async () => { + // Drive reconnection state deterministically + let reconnecting = false; + mockReconnectionManager.isReconnecting.mockImplementation( + () => reconnecting, + ); + mockReconnectionManager.startReconnection.mockImplementation(() => { + reconnecting = true; + }); + mockReconnectionManager.stopReconnection.mockImplementation(() => { + reconnecting = false; + }); + mockReconnectionManager.shouldRetry.mockReturnValue(true); + mockReconnectionManager.incrementAttempt.mockReturnValue(1); + mockReconnectionManager.calculateBackoff.mockReturnValue(0); // No delay for test + // Set up message queue with multiple messages + mockMessageQueue.dequeue + .mockReturnValueOnce({ message: 'queued-1', hints: [] }) + .mockReturnValueOnce({ message: 'queued-2', hints: [] }) + .mockReturnValueOnce({ message: 'queued-3', hints: [] }) + .mockReturnValue(undefined); + mockMessageQueue.length = 3; + mockMessageQueue.messages = [ + { message: 'queued-1', hints: [] }, + { message: 'queued-2', hints: [] }, + { message: 'queued-3', hints: [] }, + ]; + const mockChannel = createMockChannel('peer-1'); + mockChannel.msgStream.write + .mockRejectedValueOnce( + Object.assign(new Error('Connection lost'), { code: 'ECONNRESET' }), + ) // First write fails, triggering reconnection + .mockResolvedValue(undefined); // All flush writes succeed + mockConnectionFactory.dialIdempotent + .mockResolvedValueOnce(mockChannel) // Initial connection + .mockResolvedValueOnce(mockChannel); // Reconnection succeeds + const { abortableDelay } = await import('@metamask/kernel-utils'); + (abortableDelay as ReturnType).mockResolvedValue(undefined); + const { sendRemoteMessage } = await initNetwork('0x1234', {}, vi.fn()); + // Establish channel + await sendRemoteMessage('peer-1', 'initial-msg'); + // Clear resetBackoff mock before triggering reconnection to get accurate count + mockReconnectionManager.resetBackoff.mockClear(); + // Trigger reconnection via write failure + await sendRemoteMessage('peer-1', 'queued-1'); + // Wait for flush to complete (3 queued messages should be flushed) + await vi.waitFor( + () => { + // Should have flushed all 3 queued messages + expect(mockChannel.msgStream.write).toHaveBeenCalledTimes(4); // initial + 3 queued + }, + { timeout: 5000 }, + ); + const resetBackoffCallCount = + mockReconnectionManager.resetBackoff.mock.calls.length; + expect(resetBackoffCallCount).toBeLessThanOrEqual(1); + }); }); describe('stop functionality', () => { it('returns a stop function', async () => { - const { stop } = await initNetwork('0x1234', [], vi.fn()); + const { stop } = await initNetwork('0x1234', {}, vi.fn()); expect(typeof stop).toBe('function'); }); it('cleans up resources on stop', async () => { - const { stop } = await initNetwork('0x1234', [], vi.fn()); + const { stop } = await initNetwork('0x1234', {}, vi.fn()); await stop(); @@ -627,7 +723,7 @@ describe('network.initNetwork', () => { it('does not send messages after stop', async () => { const { sendRemoteMessage, stop } = await initNetwork( '0x1234', - [], + {}, vi.fn(), ); @@ -663,7 +759,7 @@ describe('network.initNetwork', () => { const { sendRemoteMessage, stop } = await initNetwork( '0x1234', - [], + {}, vi.fn(), ); @@ -686,7 +782,7 @@ describe('network.initNetwork', () => { }); it('can be called multiple times safely', async () => { - const { stop } = await initNetwork('0x1234', [], vi.fn()); + const { stop } = await initNetwork('0x1234', {}, vi.fn()); // Multiple calls should not throw await stop(); @@ -701,7 +797,7 @@ describe('network.initNetwork', () => { describe('closeConnection', () => { it('returns a closeConnection function', async () => { - const { closeConnection } = await initNetwork('0x1234', [], vi.fn()); + const { closeConnection } = await initNetwork('0x1234', {}, vi.fn()); expect(typeof closeConnection).toBe('function'); }); @@ -712,7 +808,7 @@ describe('network.initNetwork', () => { const { sendRemoteMessage, closeConnection } = await initNetwork( '0x1234', - [], + {}, vi.fn(), ); @@ -734,7 +830,7 @@ describe('network.initNetwork', () => { const { sendRemoteMessage, closeConnection } = await initNetwork( '0x1234', - [], + {}, vi.fn(), ); @@ -757,7 +853,7 @@ describe('network.initNetwork', () => { const { sendRemoteMessage, closeConnection } = await initNetwork( '0x1234', - [], + {}, vi.fn(), ); @@ -782,7 +878,7 @@ describe('network.initNetwork', () => { const { sendRemoteMessage, closeConnection } = await initNetwork( '0x1234', - [], + {}, vi.fn(), ); @@ -809,7 +905,7 @@ describe('network.initNetwork', () => { }, ); - const { closeConnection } = await initNetwork('0x1234', [], vi.fn()); + const { closeConnection } = await initNetwork('0x1234', {}, vi.fn()); // Close connection first await closeConnection('peer-1'); @@ -831,7 +927,7 @@ describe('network.initNetwork', () => { describe('reconnectPeer', () => { it('returns a reconnectPeer function', async () => { - const { reconnectPeer } = await initNetwork('0x1234', [], vi.fn()); + const { reconnectPeer } = await initNetwork('0x1234', {}, vi.fn()); expect(typeof reconnectPeer).toBe('function'); }); @@ -841,7 +937,7 @@ describe('network.initNetwork', () => { mockConnectionFactory.dialIdempotent.mockResolvedValue(mockChannel); const { sendRemoteMessage, closeConnection, reconnectPeer } = - await initNetwork('0x1234', [], vi.fn()); + await initNetwork('0x1234', {}, vi.fn()); // Establish and close connection await sendRemoteMessage('peer-1', 'msg1'); @@ -885,7 +981,7 @@ describe('network.initNetwork', () => { const { closeConnection, reconnectPeer } = await initNetwork( '0x1234', - [], + {}, vi.fn(), ); @@ -933,7 +1029,7 @@ describe('network.initNetwork', () => { const { closeConnection, reconnectPeer } = await initNetwork( '0x1234', - [], + {}, vi.fn(), ); @@ -956,7 +1052,7 @@ describe('network.initNetwork', () => { const { closeConnection, reconnectPeer } = await initNetwork( '0x1234', - [], + {}, vi.fn(), ); @@ -976,7 +1072,7 @@ describe('network.initNetwork', () => { mockConnectionFactory.dialIdempotent.mockResolvedValue(mockChannel); const { sendRemoteMessage, closeConnection, reconnectPeer } = - await initNetwork('0x1234', [], vi.fn()); + await initNetwork('0x1234', {}, vi.fn()); // Establish, close, and reconnect await sendRemoteMessage('peer-1', 'msg1'); @@ -1009,7 +1105,7 @@ describe('network.initNetwork', () => { }, ); - await initNetwork('0x1234', [], vi.fn()); + await initNetwork('0x1234', {}, vi.fn()); expect(installWakeDetector).toHaveBeenCalled(); @@ -1026,7 +1122,7 @@ describe('network.initNetwork', () => { cleanupFn, ); - const { stop } = await initNetwork('0x1234', [], vi.fn()); + const { stop } = await initNetwork('0x1234', {}, vi.fn()); await stop(); @@ -1048,7 +1144,7 @@ describe('network.initNetwork', () => { return mockChannel; }); - const { sendRemoteMessage } = await initNetwork('0x1234', [], vi.fn()); + const { sendRemoteMessage } = await initNetwork('0x1234', {}, vi.fn()); await sendRemoteMessage('peer-1', 'msg'); @@ -1101,7 +1197,7 @@ describe('network.initNetwork', () => { return reconChannel; }); - const { sendRemoteMessage } = await initNetwork('0x1234', [], vi.fn()); + const { sendRemoteMessage } = await initNetwork('0x1234', {}, vi.fn()); // Trigger first connection loss (this starts reconnection) await sendRemoteMessage('peer-1', 'msg-1'); @@ -1132,7 +1228,7 @@ describe('network.initNetwork', () => { new Error('Dial failed'), ); - const { sendRemoteMessage } = await initNetwork('0x1234', [], vi.fn()); + const { sendRemoteMessage } = await initNetwork('0x1234', {}, vi.fn()); await sendRemoteMessage('peer-1', 'msg'); @@ -1160,7 +1256,7 @@ describe('network.initNetwork', () => { .mockResolvedValueOnce(mockChannel) // initial connection .mockRejectedValueOnce(new Error('Permanent failure')); // non-retryable during reconnection - const { sendRemoteMessage } = await initNetwork('0x1234', [], vi.fn()); + const { sendRemoteMessage } = await initNetwork('0x1234', {}, vi.fn()); // Establish channel await sendRemoteMessage('peer-1', 'msg1'); @@ -1193,10 +1289,10 @@ describe('network.initNetwork', () => { mockReconnectionManager.startReconnection.mockImplementation(() => { reconnecting = true; }); - // First call should return true (to enter loop), then false (max attempts) + // First call should return true (to enter loop), then false when checking after flush failure mockReconnectionManager.shouldRetry .mockReturnValueOnce(true) // Enter loop - .mockReturnValue(false); // Max attempts reached + .mockReturnValueOnce(false); // Max attempts reached (checked after flush failure) mockReconnectionManager.stopReconnection.mockImplementation(() => { reconnecting = false; }); @@ -1204,15 +1300,22 @@ describe('network.initNetwork', () => { const { abortableDelay } = await import('@metamask/kernel-utils'); (abortableDelay as ReturnType).mockResolvedValue(undefined); + // Set up queue with messages that will fail during flush + mockMessageQueue.dequeue + .mockReturnValueOnce({ message: 'queued-msg', hints: [] }) + .mockReturnValue(undefined); + mockMessageQueue.length = 1; + mockMessageQueue.messages = [{ message: 'queued-msg', hints: [] }]; + const mockChannel = createMockChannel('peer-1'); mockChannel.msgStream.write.mockRejectedValue( Object.assign(new Error('Connection lost'), { code: 'ECONNRESET' }), ); mockConnectionFactory.dialIdempotent .mockResolvedValueOnce(mockChannel) // initial connection - .mockResolvedValue(mockChannel); // reconnection attempts (won't succeed) + .mockResolvedValue(mockChannel); // reconnection attempts (dial succeeds, flush fails) - const { sendRemoteMessage } = await initNetwork('0x1234', [], vi.fn()); + const { sendRemoteMessage } = await initNetwork('0x1234', {}, vi.fn()); // Establish channel await sendRemoteMessage('peer-1', 'msg1'); @@ -1230,11 +1333,198 @@ describe('network.initNetwork', () => { }); }); + it('calls onRemoteGiveUp when max attempts reached', async () => { + const onRemoteGiveUp = vi.fn(); + let reconnecting = false; + mockReconnectionManager.isReconnecting.mockImplementation( + () => reconnecting, + ); + mockReconnectionManager.startReconnection.mockImplementation(() => { + reconnecting = true; + }); + mockReconnectionManager.shouldRetry + .mockReturnValueOnce(true) + .mockReturnValueOnce(false); // Max attempts reached (checked after flush failure) + mockReconnectionManager.stopReconnection.mockImplementation(() => { + reconnecting = false; + }); + + const { abortableDelay } = await import('@metamask/kernel-utils'); + (abortableDelay as ReturnType).mockResolvedValue(undefined); + + // Set up queue with messages that will fail during flush + mockMessageQueue.dequeue + .mockReturnValueOnce({ message: 'queued-msg', hints: [] }) + .mockReturnValue(undefined); + mockMessageQueue.length = 1; + mockMessageQueue.messages = [{ message: 'queued-msg', hints: [] }]; + + const mockChannel = createMockChannel('peer-1'); + mockChannel.msgStream.write.mockRejectedValue( + Object.assign(new Error('Connection lost'), { code: 'ECONNRESET' }), + ); + mockConnectionFactory.dialIdempotent + .mockResolvedValueOnce(mockChannel) + .mockResolvedValue(mockChannel); + + const { sendRemoteMessage } = await initNetwork( + '0x1234', + {}, + vi.fn(), + onRemoteGiveUp, + ); + + await sendRemoteMessage('peer-1', 'msg1'); + await sendRemoteMessage('peer-1', 'msg2'); + + await vi.waitFor(() => { + expect(onRemoteGiveUp).toHaveBeenCalledWith('peer-1'); + }); + }); + + it('respects maxRetryAttempts limit even when flush operations occur', async () => { + const maxRetryAttempts = 3; + const onRemoteGiveUp = vi.fn(); + let attemptCount = 0; + let reconnecting = false; + mockReconnectionManager.isReconnecting.mockImplementation( + () => reconnecting, + ); + mockReconnectionManager.startReconnection.mockImplementation(() => { + reconnecting = true; + attemptCount = 0; // Reset on start + }); + mockReconnectionManager.incrementAttempt.mockImplementation(() => { + attemptCount += 1; + return attemptCount; + }); + mockReconnectionManager.shouldRetry.mockImplementation( + (_peerId: string, maxAttempts: number) => { + if (maxAttempts === 0) { + return true; + } + // shouldRetry should return false when attemptCount >= maxAttempts + return attemptCount < maxAttempts; + }, + ); + mockReconnectionManager.calculateBackoff.mockReturnValue(0); // No delay for test + mockReconnectionManager.resetBackoff.mockImplementation(() => { + attemptCount = 0; // Reset attempt count + }); + mockReconnectionManager.stopReconnection.mockImplementation(() => { + reconnecting = false; + }); + const { abortableDelay } = await import('@metamask/kernel-utils'); + (abortableDelay as ReturnType).mockResolvedValue(undefined); + const mockChannel = createMockChannel('peer-1'); + // All writes fail to trigger reconnection + mockChannel.msgStream.write.mockRejectedValue( + Object.assign(new Error('Connection lost'), { code: 'ECONNRESET' }), + ); + // All reconnection attempts fail (dial succeeds but flush fails) + mockConnectionFactory.dialIdempotent.mockResolvedValue(mockChannel); + // Set up queue with messages that will be flushed during reconnection + // Each reconnection attempt will try to flush these messages, and they will fail + const queuedMsg1 = { message: 'queued-1', hints: [] }; + const queuedMsg2 = { message: 'queued-2', hints: [] }; + // dequeue should return messages for each flush attempt (each reconnection) + mockMessageQueue.dequeue.mockImplementation(() => { + // Return messages in order, then undefined + if (mockMessageQueue.messages.length > 0) { + return mockMessageQueue.messages.shift(); + } + return undefined; + }); + mockMessageQueue.length = 2; + mockMessageQueue.messages = [queuedMsg1, queuedMsg2]; + // When replaceAll is called (after flush failure), restore the messages + mockMessageQueue.replaceAll.mockImplementation((messages) => { + mockMessageQueue.messages = [...messages]; + mockMessageQueue.length = messages.length; + }); + const { sendRemoteMessage } = await initNetwork( + '0x1234', + { maxRetryAttempts }, + vi.fn(), + onRemoteGiveUp, + ); + // Establish channel + await sendRemoteMessage('peer-1', 'msg1'); + // Trigger reconnection via write failure + await sendRemoteMessage('peer-1', 'msg2'); + // Wait for maxRetryAttempts to be reached + await vi.waitFor( + () => { + // Should have called incrementAttempt exactly maxRetryAttempts times + expect( + mockReconnectionManager.incrementAttempt, + ).toHaveBeenCalledTimes(maxRetryAttempts); + // Should have stopped reconnection and called onRemoteGiveUp + expect(mockReconnectionManager.stopReconnection).toHaveBeenCalledWith( + 'peer-1', + ); + expect(onRemoteGiveUp).toHaveBeenCalledWith('peer-1'); + expect(mockMessageQueue.clear).toHaveBeenCalled(); + }, + { timeout: 10000 }, + ); + const resetBackoffCalls = mockReconnectionManager.resetBackoff.mock.calls; + expect(resetBackoffCalls).toHaveLength(0); + }, 10000); + + it('calls onRemoteGiveUp when non-retryable error occurs', async () => { + const onRemoteGiveUp = vi.fn(); + let reconnecting = false; + mockReconnectionManager.isReconnecting.mockImplementation( + () => reconnecting, + ); + mockReconnectionManager.startReconnection.mockImplementation(() => { + reconnecting = true; + }); + mockReconnectionManager.shouldRetry.mockReturnValue(true); + mockReconnectionManager.incrementAttempt.mockReturnValue(1); + mockReconnectionManager.calculateBackoff.mockReturnValue(0); + mockReconnectionManager.stopReconnection.mockImplementation(() => { + reconnecting = false; + }); + + const { abortableDelay } = await import('@metamask/kernel-utils'); + (abortableDelay as ReturnType).mockResolvedValue(undefined); + + const { isRetryableNetworkError } = await import( + '@metamask/kernel-errors' + ); + vi.mocked(isRetryableNetworkError).mockReturnValue(false); + + const mockChannel = createMockChannel('peer-1'); + mockChannel.msgStream.write.mockRejectedValue( + Object.assign(new Error('Connection lost'), { code: 'ECONNRESET' }), + ); + mockConnectionFactory.dialIdempotent + .mockResolvedValueOnce(mockChannel) + .mockRejectedValueOnce(new Error('Non-retryable error')); + + const { sendRemoteMessage } = await initNetwork( + '0x1234', + {}, + vi.fn(), + onRemoteGiveUp, + ); + + await sendRemoteMessage('peer-1', 'msg1'); + await sendRemoteMessage('peer-1', 'msg2'); + + await vi.waitFor(() => { + expect(onRemoteGiveUp).toHaveBeenCalledWith('peer-1'); + expect(mockMessageQueue.clear).toHaveBeenCalled(); + }); + }); + it('resets backoff on successful message send', async () => { const mockChannel = createMockChannel('peer-1'); mockConnectionFactory.dialIdempotent.mockResolvedValue(mockChannel); - const { sendRemoteMessage } = await initNetwork('0x1234', [], vi.fn()); + const { sendRemoteMessage } = await initNetwork('0x1234', {}, vi.fn()); await sendRemoteMessage('peer-1', 'msg'); @@ -1251,7 +1541,7 @@ describe('network.initNetwork', () => { }, ); - await initNetwork('0x1234', [], vi.fn()); + await initNetwork('0x1234', {}, vi.fn()); const mockChannel = createMockChannel('inbound-peer'); const messageBuffer = new TextEncoder().encode('inbound-msg'); @@ -1279,7 +1569,7 @@ describe('network.initNetwork', () => { mockReconnectionManager.isReconnecting.mockReturnValue(true); const hints = ['/dns4/hint.example/tcp/443/wss/p2p/hint']; - const { sendRemoteMessage } = await initNetwork('0x1234', [], vi.fn()); + const { sendRemoteMessage } = await initNetwork('0x1234', {}, vi.fn()); await sendRemoteMessage('peer-1', 'msg', hints); @@ -1322,7 +1612,7 @@ describe('network.initNetwork', () => { .mockReturnValue(undefined); mockMessageQueue.length = 2; - const { sendRemoteMessage } = await initNetwork('0x1234', [], vi.fn()); + const { sendRemoteMessage } = await initNetwork('0x1234', {}, vi.fn()); // Establish initial channel await sendRemoteMessage('peer-1', 'initial'); @@ -1379,7 +1669,7 @@ describe('network.initNetwork', () => { .mockResolvedValueOnce(mockChannel) // initial connection .mockResolvedValueOnce(mockChannel); // reconnection - const { sendRemoteMessage } = await initNetwork('0x1234', [], vi.fn()); + const { sendRemoteMessage } = await initNetwork('0x1234', {}, vi.fn()); // Establish channel await sendRemoteMessage('peer-1', 'msg1'); @@ -1449,7 +1739,7 @@ describe('network.initNetwork', () => { .mockResolvedValueOnce(mockChannel1) // initial connection .mockResolvedValueOnce(mockChannel2); // reconnection after flush failure - const { sendRemoteMessage } = await initNetwork('0x1234', [], vi.fn()); + const { sendRemoteMessage } = await initNetwork('0x1234', {}, vi.fn()); // Establish channel await sendRemoteMessage('peer-1', 'msg1'); @@ -1468,4 +1758,65 @@ describe('network.initNetwork', () => { }); }); }); + + describe('updateChannelHints', () => { + it('merges hints from queued messages during flush', async () => { + // Drive reconnection state deterministically + let reconnecting = false; + mockReconnectionManager.isReconnecting.mockImplementation( + () => reconnecting, + ); + mockReconnectionManager.startReconnection.mockImplementation(() => { + reconnecting = true; + }); + mockReconnectionManager.stopReconnection.mockImplementation(() => { + reconnecting = false; + }); + + const mockChannel = createMockChannel('peer-1', ['hint1', 'hint2']); + + mockConnectionFactory.dialIdempotent + .mockResolvedValueOnce(mockChannel) // initial connection + .mockResolvedValueOnce(mockChannel); // reconnection + + // Set up queue with messages that have hints + mockMessageQueue.messages = [ + { message: 'queued-1', hints: ['hint2', 'hint3'] }, + ]; + mockMessageQueue.dequeue + .mockReturnValueOnce({ message: 'queued-1', hints: ['hint2', 'hint3'] }) + .mockReturnValue(undefined); + mockMessageQueue.length = 1; + + const { sendRemoteMessage } = await initNetwork('0x1234', {}, vi.fn()); + + // Establish channel + await sendRemoteMessage('peer-1', 'msg1'); + + // Trigger reconnection via write failure + mockChannel.msgStream.write + .mockRejectedValueOnce( + Object.assign(new Error('Connection lost'), { code: 'ECONNRESET' }), + ) + .mockResolvedValue(undefined); + + await sendRemoteMessage('peer-1', 'msg2'); + + await vi.waitFor(() => { + // Hints should be merged: ['hint1', 'hint2', 'hint3'] + expect(mockChannel.hints).toContain('hint1'); + expect(mockChannel.hints).toContain('hint2'); + expect(mockChannel.hints).toContain('hint3'); + expect(mockChannel.hints).toHaveLength(3); + }); + }); + + it('does not update hints if channel does not exist', async () => { + const { sendRemoteMessage } = await initNetwork('0x1234', {}, vi.fn()); + + // Sending message to non-existent peer should not crash + const promise = sendRemoteMessage('non-existent-peer', 'msg', ['hint1']); + expect(await promise).toBeUndefined(); + }); + }); }); diff --git a/packages/ocap-kernel/src/remotes/network.ts b/packages/ocap-kernel/src/remotes/network.ts index 9c8db86a2..6bca04e7a 100644 --- a/packages/ocap-kernel/src/remotes/network.ts +++ b/packages/ocap-kernel/src/remotes/network.ts @@ -16,30 +16,42 @@ import type { SendRemoteMessage, StopRemoteComms, Channel, + OnRemoteGiveUp, + RemoteCommsOptions, } from './types.ts'; -/** Upper bound for queued outbound messages while reconnecting */ -const MAX_QUEUE = 200; +/** Default upper bound for queued outbound messages while reconnecting */ +const DEFAULT_MAX_QUEUE = 200; /** * Initialize the remote comm system with information that must be provided by the kernel. * * @param keySeed - Seed value for key generation, in the form of a hex-encoded string. - * @param knownRelays - PeerIds/Multiaddrs of known message relays. + * @param options - Options for remote communications initialization. + * @param options.relays - PeerIds/Multiaddrs of known message relays. + * @param options.maxRetryAttempts - Maximum number of reconnection attempts. 0 = infinite (default). + * @param options.maxQueue - Maximum number of messages to queue per peer while reconnecting (default: 200). * @param remoteMessageHandler - Handler to be called when messages are received from elsewhere. + * @param onRemoteGiveUp - Optional callback to be called when we give up on a remote (after max retries or non-retryable error). * * @returns a function to send messages **and** a `stop()` to cancel/release everything. */ export async function initNetwork( keySeed: string, - knownRelays: string[], + options: RemoteCommsOptions, remoteMessageHandler: RemoteMessageHandler, + onRemoteGiveUp?: OnRemoteGiveUp, ): Promise<{ sendRemoteMessage: SendRemoteMessage; stop: StopRemoteComms; closeConnection: (peerId: string) => Promise; reconnectPeer: (peerId: string, hints?: string[]) => Promise; }> { + const { + relays = [], + maxRetryAttempts, + maxQueue = DEFAULT_MAX_QUEUE, + } = options; let cleanupWakeDetector: (() => void) | undefined; const stopController = new AbortController(); const { signal } = stopController; @@ -50,9 +62,10 @@ export async function initNetwork( const intentionallyClosed = new Set(); // Track peers that intentionally closed connections const connectionFactory = await ConnectionFactory.make( keySeed, - knownRelays, + relays, logger, signal, + maxRetryAttempts, ); /** @@ -97,7 +110,7 @@ export async function initNetwork( function getMessageQueue(peerId: string): MessageQueue { let queue = messageQueues.get(peerId); if (!queue) { - queue = new MessageQueue(MAX_QUEUE); + queue = new MessageQueue(maxQueue); messageQueues.set(peerId, queue); } return queue; @@ -210,7 +223,7 @@ export async function initNetwork( async function attemptReconnection( peerId: string, hints: string[] = [], - maxAttempts = DEFAULT_MAX_RETRY_ATTEMPTS, + maxAttempts = maxRetryAttempts ?? DEFAULT_MAX_RETRY_ATTEMPTS, ): Promise { const queue = getMessageQueue(peerId); @@ -221,6 +234,7 @@ export async function initNetwork( ); reconnectionManager.stopReconnection(peerId); queue.clear(); + onRemoteGiveUp?.(peerId); return; } @@ -256,9 +270,6 @@ export async function initNetwork( logger.log(`${peerId}:: reconnection successful`); - // Reset backoff but keep reconnection active until flush completes - reconnectionManager.resetBackoff(peerId); - // Start reading from the new channel readChannel(channel).catch((problem) => { outputError(peerId, `reading channel to`, problem); @@ -267,15 +278,16 @@ export async function initNetwork( // Flush queued messages await flushQueuedMessages(peerId, channel, queue); - // Check if reconnection was restarted during flush (e.g., due to flush errors) - if (reconnectionManager.isReconnecting(peerId)) { + // Check if channel was deleted during flush (e.g., due to flush errors) + if (!channels.has(peerId)) { logger.log( - `${peerId}:: reconnection restarted during flush, continuing loop`, + `${peerId}:: channel deleted during flush, continuing loop`, ); continue; // Continue the reconnection loop } - // Only stop reconnection after successful flush + // Only reset backoff and stop reconnection after successful flush + reconnectionManager.resetBackoff(peerId); reconnectionManager.stopReconnection(peerId); return; // success } catch (problem) { @@ -287,6 +299,7 @@ export async function initNetwork( outputError(peerId, `non-retryable failure`, problem); reconnectionManager.stopReconnection(peerId); queue.clear(); + onRemoteGiveUp?.(peerId); return; } outputError(peerId, `reconnection attempt ${nextAttempt}`, problem); @@ -331,7 +344,6 @@ export async function initNetwork( try { logger.log(`${peerId}:: send (queued) ${queuedMsg.message}`); await channel.msgStream.write(fromString(queuedMsg.message)); - reconnectionManager.resetBackoff(peerId); } catch (problem) { outputError(peerId, `sending queued message`, problem); // Preserve the failed message and all remaining messages @@ -375,7 +387,7 @@ export async function initNetwork( queue.enqueue(message, hints); logger.log( `${targetPeerId}:: queueing message during reconnection ` + - `(${queue.length}/${MAX_QUEUE}): ${message}`, + `(${queue.length}/${maxQueue}): ${message}`, ); return; } @@ -394,7 +406,7 @@ export async function initNetwork( queue.enqueue(message, hints); logger.log( `${targetPeerId}:: reconnection started during dial, queueing message ` + - `(${queue.length}/${MAX_QUEUE}): ${message}`, + `(${queue.length}/${maxQueue}): ${message}`, ); return; } diff --git a/packages/ocap-kernel/src/remotes/remote-comms.test.ts b/packages/ocap-kernel/src/remotes/remote-comms.test.ts index 17166d8d3..175568ad3 100644 --- a/packages/ocap-kernel/src/remotes/remote-comms.test.ts +++ b/packages/ocap-kernel/src/remotes/remote-comms.test.ts @@ -1,6 +1,7 @@ import { generateKeyPairFromSeed } from '@libp2p/crypto/keys'; import { peerIdFromPrivateKey } from '@libp2p/peer-id'; import { fromHex } from '@metamask/kernel-utils'; +import type { Logger } from '@metamask/logger'; import { describe, it, expect, vi, beforeEach } from 'vitest'; import { @@ -45,15 +46,12 @@ describe('remote-comms', () => { mockKernelStore, mockPlatformServices, mockRemoteMessageHandler, - testRelays, + { relays: testRelays }, ); expect(remoteComms).toHaveProperty('getPeerId'); expect(remoteComms).toHaveProperty('issueOcapURL'); expect(remoteComms).toHaveProperty('redeemLocalOcapURL'); expect(remoteComms).toHaveProperty('sendRemoteMessage'); - expect(remoteComms).toHaveProperty('stopRemoteComms'); - expect(remoteComms).toHaveProperty('closeConnection'); - expect(remoteComms).toHaveProperty('reconnectPeer'); const keySeed = mockKernelStore.kv.get('keySeed'); expect(keySeed).toBe( @@ -92,6 +90,13 @@ describe('remote-comms', () => { 'your message here', [], ); + + await remoteComms.sendRemoteMessage('peer1', 'msg', ['hint1', 'hint2']); + expect(mockPlatformServices.sendRemoteMessage).toHaveBeenCalledWith( + 'peer1', + 'msg', + ['hint1', 'hint2'], + ); }); it('honors pre-existing comms initialization parameters when present', async () => { @@ -110,177 +115,185 @@ describe('remote-comms', () => { expect(remoteComms).toHaveProperty('issueOcapURL'); expect(remoteComms).toHaveProperty('redeemLocalOcapURL'); expect(remoteComms).toHaveProperty('sendRemoteMessage'); - expect(remoteComms).toHaveProperty('stopRemoteComms'); - expect(remoteComms).toHaveProperty('closeConnection'); - expect(remoteComms).toHaveProperty('reconnectPeer'); expect(mockKernelStore.kv.get('peerId')).toBe(mockPeerId); expect(remoteComms.getPeerId()).toBe(mockPeerId); expect(mockKernelStore.kv.get('keySeed')).toBe(mockKeySeed); expect(mockKernelStore.kv.get('ocapURLKey')).toBe(mockOcapURLKey); }); - }); - describe('stopRemoteComms', () => { - it('calls platformServices.stopRemoteComms', async () => { - const remoteComms = await initRemoteComms( + it('passes options object to platformServices.initializeRemoteComms', async () => { + const options = { + relays: ['/dns4/relay.example/tcp/443/wss/p2p/relay'], + maxRetryAttempts: 5, + maxQueue: 100, + }; + await initRemoteComms( mockKernelStore, mockPlatformServices, mockRemoteMessageHandler, + options, ); - await remoteComms.stopRemoteComms(); - expect(mockPlatformServices.stopRemoteComms).toHaveBeenCalledOnce(); - }); - - it('is a bound function from platformServices', async () => { - const remoteComms = await initRemoteComms( - mockKernelStore, - mockPlatformServices, + expect(mockPlatformServices.initializeRemoteComms).toHaveBeenCalledWith( + expect.any(String), // keySeed + expect.objectContaining({ + relays: options.relays, + maxRetryAttempts: options.maxRetryAttempts, + maxQueue: options.maxQueue, + }), mockRemoteMessageHandler, + undefined, // onRemoteGiveUp ); - expect(typeof remoteComms.stopRemoteComms).toBe('function'); - await remoteComms.stopRemoteComms(); - expect(mockPlatformServices.stopRemoteComms).toHaveBeenCalled(); }); - it('works after sending messages', async () => { - const remoteComms = await initRemoteComms( + it('uses getKnownRelays when options.relays is empty', async () => { + const storedRelays = [ + '/dns4/stored-relay1.example/tcp/443/wss/p2p/relay1', + '/dns4/stored-relay2.example/tcp/443/wss/p2p/relay2', + ]; + mockKernelStore.kv.set('knownRelays', JSON.stringify(storedRelays)); + await initRemoteComms( mockKernelStore, mockPlatformServices, mockRemoteMessageHandler, + {}, // empty options ); - await remoteComms.sendRemoteMessage('peer1', 'msg1'); - await remoteComms.sendRemoteMessage('peer2', 'msg2'); - await remoteComms.stopRemoteComms(); - expect(mockPlatformServices.stopRemoteComms).toHaveBeenCalledOnce(); - }); - - it('works after issuing ocap URLs', async () => { - const remoteComms = await initRemoteComms( - mockKernelStore, - mockPlatformServices, + expect(mockPlatformServices.initializeRemoteComms).toHaveBeenCalledWith( + expect.any(String), + expect.objectContaining({ + relays: storedRelays, + }), mockRemoteMessageHandler, + undefined, ); - await remoteComms.issueOcapURL('kref1'); - await remoteComms.issueOcapURL('kref2'); - await remoteComms.stopRemoteComms(); - expect(mockPlatformServices.stopRemoteComms).toHaveBeenCalledOnce(); }); - }); - describe('closeConnection', () => { - it('calls platformServices.closeConnection', async () => { - const remoteComms = await initRemoteComms( + it('passes onRemoteGiveUp callback to platformServices', async () => { + const onRemoteGiveUp = vi.fn(); + await initRemoteComms( mockKernelStore, mockPlatformServices, mockRemoteMessageHandler, + {}, + undefined, // logger + undefined, // keySeed + onRemoteGiveUp, ); - await remoteComms.closeConnection('peer123'); - expect(mockPlatformServices.closeConnection).toHaveBeenCalledWith( - 'peer123', - ); - }); - - it('is a bound function from platformServices', async () => { - const remoteComms = await initRemoteComms( - mockKernelStore, - mockPlatformServices, + expect(mockPlatformServices.initializeRemoteComms).toHaveBeenCalledWith( + expect.any(String), + expect.any(Object), mockRemoteMessageHandler, + onRemoteGiveUp, ); - expect(typeof remoteComms.closeConnection).toBe('function'); - await remoteComms.closeConnection('peer123'); - expect(mockPlatformServices.closeConnection).toHaveBeenCalled(); }); - it('works after sending messages', async () => { - const remoteComms = await initRemoteComms( + it('uses provided keySeed when creating new peer', async () => { + const providedKeySeed = + '1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef'; + await initRemoteComms( mockKernelStore, mockPlatformServices, mockRemoteMessageHandler, + {}, + undefined, + providedKeySeed, ); - await remoteComms.sendRemoteMessage('peer1', 'msg1'); - await remoteComms.closeConnection('peer1'); - expect(mockPlatformServices.closeConnection).toHaveBeenCalledWith( - 'peer1', + expect(mockKernelStore.kv.get('keySeed')).toBe(providedKeySeed); + const peerId = mockKernelStore.kv.get('peerId'); + expect(peerId).toBeDefined(); + // Verify peerId matches the provided keySeed + const keyPair = await generateKeyPairFromSeed( + 'Ed25519', + fromHex(providedKeySeed), ); + expect(peerId).toBe(peerIdFromPrivateKey(keyPair).toString()); }); - it('works after issuing ocap URLs', async () => { - const remoteComms = await initRemoteComms( + it('calls logger.log when existing peer id is found', async () => { + const mockLogger = { + log: vi.fn(), + error: vi.fn(), + }; + const mockPeerId = 'existing-peer-id'; + const mockKeySeed = 'abcdef'; + mockKernelStore.kv.set('peerId', mockPeerId); + mockKernelStore.kv.set('keySeed', mockKeySeed); + await initRemoteComms( mockKernelStore, mockPlatformServices, mockRemoteMessageHandler, + {}, + mockLogger as unknown as Logger, ); - await remoteComms.issueOcapURL('kref1'); - await remoteComms.closeConnection('peer123'); - expect(mockPlatformServices.closeConnection).toHaveBeenCalledWith( - 'peer123', + expect(mockLogger.log).toHaveBeenCalledWith( + `comms init: existing peer id: ${mockPeerId}`, ); }); - }); - describe('reconnectPeer', () => { - it('calls platformServices.reconnectPeer with hints', async () => { - const remoteComms = await initRemoteComms( + it('calls logger.log when new peer id is created', async () => { + const mockLogger = { + log: vi.fn(), + error: vi.fn(), + }; + await initRemoteComms( mockKernelStore, mockPlatformServices, mockRemoteMessageHandler, + {}, + mockLogger as unknown as Logger, ); - await remoteComms.reconnectPeer('peer123', ['relay1', 'relay2']); - expect(mockPlatformServices.reconnectPeer).toHaveBeenCalledWith( - 'peer123', - ['relay1', 'relay2'], + const peerId = mockKernelStore.kv.get('peerId'); + expect(mockLogger.log).toHaveBeenCalledWith( + `comms init: new peer id: ${peerId}`, ); }); - it('calls platformServices.reconnectPeer with empty hints when not provided', async () => { - const remoteComms = await initRemoteComms( + it('calls logger.log with relays', async () => { + const mockLogger = { + log: vi.fn(), + error: vi.fn(), + }; + const testRelays = ['/dns4/relay.example/tcp/443/wss/p2p/relay']; + await initRemoteComms( mockKernelStore, mockPlatformServices, mockRemoteMessageHandler, + { relays: testRelays }, + mockLogger as unknown as Logger, ); - await remoteComms.reconnectPeer('peer123'); - expect(mockPlatformServices.reconnectPeer).toHaveBeenCalledWith( - 'peer123', + expect(mockLogger.log).toHaveBeenCalledWith( + `relays: ${JSON.stringify(testRelays)}`, ); - // The default parameter [] is handled by the actual implementation - // The mock receives undefined for the second argument when not provided }); - it('is a bound function from platformServices', async () => { - const remoteComms = await initRemoteComms( + it('saves relays to KV store when provided', async () => { + const testRelays = [ + '/dns4/relay1.example/tcp/443/wss/p2p/relay1', + '/dns4/relay2.example/tcp/443/wss/p2p/relay2', + ]; + await initRemoteComms( mockKernelStore, mockPlatformServices, mockRemoteMessageHandler, + { relays: testRelays }, ); - expect(typeof remoteComms.reconnectPeer).toBe('function'); - await remoteComms.reconnectPeer('peer123'); - expect(mockPlatformServices.reconnectPeer).toHaveBeenCalled(); - }); - - it('works after closing connection', async () => { - const remoteComms = await initRemoteComms( - mockKernelStore, - mockPlatformServices, - mockRemoteMessageHandler, + expect(mockKernelStore.kv.get('knownRelays')).toBe( + JSON.stringify(testRelays), ); - await remoteComms.closeConnection('peer1'); - await remoteComms.reconnectPeer('peer1', ['relay1']); - expect(mockPlatformServices.reconnectPeer).toHaveBeenCalledWith('peer1', [ - 'relay1', - ]); }); - it('works after sending messages', async () => { - const remoteComms = await initRemoteComms( + it('does not save relays to KV store when empty', async () => { + const storedRelays = ['/dns4/stored-relay.example/tcp/443/wss/p2p/relay']; + mockKernelStore.kv.set('knownRelays', JSON.stringify(storedRelays)); + await initRemoteComms( mockKernelStore, mockPlatformServices, mockRemoteMessageHandler, + {}, // empty relays + ); + // Should not overwrite existing relays + expect(mockKernelStore.kv.get('knownRelays')).toBe( + JSON.stringify(storedRelays), ); - await remoteComms.sendRemoteMessage('peer1', 'msg1'); - await remoteComms.reconnectPeer('peer1'); - expect(mockPlatformServices.reconnectPeer).toHaveBeenCalledWith('peer1'); - // The default parameter [] is handled by the actual implementation - // The mock receives undefined for the second argument when not provided }); }); @@ -398,5 +411,121 @@ describe('remote-comms', () => { remoteComms.redeemLocalOcapURL(corruptedURL), ).rejects.toThrow('ocapURL has bad object reference'); }); + + it('calls logger.error when decryption fails', async () => { + const mockLogger = { + log: vi.fn(), + error: vi.fn(), + }; + const remoteComms = await initRemoteComms( + mockKernelStore, + mockPlatformServices, + mockRemoteMessageHandler, + {}, + mockLogger as unknown as Logger, + ); + + const validURL = await remoteComms.issueOcapURL('test-kref'); + const peerId = remoteComms.getPeerId(); + const { oid } = parseOcapURL(validURL); + const corruptedOid = `${oid.slice(0, -5)}zzzzz`; + const corruptedURL = `ocap:${corruptedOid}@${peerId}`; + + await expect( + remoteComms.redeemLocalOcapURL(corruptedURL), + ).rejects.toThrow('ocapURL has bad object reference'); + + expect(mockLogger.error).toHaveBeenCalledWith( + 'problem deciphering encoded kref: ', + expect.any(Error), + ); + }); + + it('handles issueOcapURL with short kref', async () => { + const remoteComms = await initRemoteComms( + mockKernelStore, + mockPlatformServices, + mockRemoteMessageHandler, + ); + + const shortKref = 'abc'; + const ocapURL = await remoteComms.issueOcapURL(shortKref); + const kref = await remoteComms.redeemLocalOcapURL(ocapURL); + expect(kref).toBe(shortKref); + }); + + it('handles issueOcapURL with empty kref', async () => { + const remoteComms = await initRemoteComms( + mockKernelStore, + mockPlatformServices, + mockRemoteMessageHandler, + ); + + const emptyKref = ''; + const ocapURL = await remoteComms.issueOcapURL(emptyKref); + const kref = await remoteComms.redeemLocalOcapURL(ocapURL); + expect(kref).toBe(emptyKref); + }); + + it('handles issueOcapURL with long kref', async () => { + const remoteComms = await initRemoteComms( + mockKernelStore, + mockPlatformServices, + mockRemoteMessageHandler, + ); + + const longKref = 'a'.repeat(100); + const ocapURL = await remoteComms.issueOcapURL(longKref); + const kref = await remoteComms.redeemLocalOcapURL(ocapURL); + expect(kref).toBe(longKref); + }); + + it('handles issueOcapURL with kref containing special characters', async () => { + const remoteComms = await initRemoteComms( + mockKernelStore, + mockPlatformServices, + mockRemoteMessageHandler, + ); + + const specialKref = 'kref-with-special-chars-!@#$%^&*()'; + const ocapURL = await remoteComms.issueOcapURL(specialKref); + const kref = await remoteComms.redeemLocalOcapURL(ocapURL); + expect(kref).toBe(specialKref); + }); + + it('includes knownRelays in issued ocap URLs', async () => { + const testRelays = [ + '/dns4/relay1.example/tcp/443/wss/p2p/relay1', + '/dns4/relay2.example/tcp/443/wss/p2p/relay2', + ]; + const remoteComms = await initRemoteComms( + mockKernelStore, + mockPlatformServices, + mockRemoteMessageHandler, + { relays: testRelays }, + ); + + const ocapURL = await remoteComms.issueOcapURL('test-kref'); + const { hints } = parseOcapURL(ocapURL); + expect(hints).toStrictEqual(testRelays); + }); + + it('includes stored relays in issued ocap URLs when options.relays is empty', async () => { + const storedRelays = [ + '/dns4/stored-relay1.example/tcp/443/wss/p2p/relay1', + '/dns4/stored-relay2.example/tcp/443/wss/p2p/relay2', + ]; + mockKernelStore.kv.set('knownRelays', JSON.stringify(storedRelays)); + const remoteComms = await initRemoteComms( + mockKernelStore, + mockPlatformServices, + mockRemoteMessageHandler, + {}, + ); + + const ocapURL = await remoteComms.issueOcapURL('test-kref'); + const { hints } = parseOcapURL(ocapURL); + expect(hints).toStrictEqual(storedRelays); + }); }); }); diff --git a/packages/ocap-kernel/src/remotes/remote-comms.ts b/packages/ocap-kernel/src/remotes/remote-comms.ts index 8e9d96b93..87ca9c1ce 100644 --- a/packages/ocap-kernel/src/remotes/remote-comms.ts +++ b/packages/ocap-kernel/src/remotes/remote-comms.ts @@ -8,7 +8,12 @@ import { base58btc } from 'multiformats/bases/base58'; import type { KernelStore } from '../store/index.ts'; import type { PlatformServices } from '../types.ts'; -import type { RemoteComms, RemoteMessageHandler } from './types.ts'; +import type { + RemoteComms, + RemoteMessageHandler, + OnRemoteGiveUp, + RemoteCommsOptions, +} from './types.ts'; export type OcapURLParts = { oid: string; @@ -94,9 +99,10 @@ export function getKnownRelays(kv: KVStore): string[] { * @param platformServices - The platform services, for accessing network I/O * operations that are not available within the web worker that the kernel runs in. * @param remoteMessageHandler - Handler to process received inbound communcations. - * @param relays - The known relays to use for the remote comms object. + * @param options - Options for remote communications initialization. * @param logger - The logger to use. * @param keySeed - Optional seed for libp2p key generation. + * @param onRemoteGiveUp - Optional callback to be called when we give up on a remote. * * @returns the initialized remote comms object. */ @@ -104,16 +110,18 @@ export async function initRemoteComms( kernelStore: KernelStore, platformServices: PlatformServices, remoteMessageHandler: RemoteMessageHandler, - relays?: string[], + options: RemoteCommsOptions = {}, logger?: Logger, keySeed?: string, + onRemoteGiveUp?: OnRemoteGiveUp, ): Promise { let peerId: string; let ocapURLKey: Uint8Array; const encoder = new TextEncoder(); const decoder = new TextDecoder(); const { kv } = kernelStore; - if (relays && relays.length > 0) { + const { relays = [] } = options; + if (relays.length > 0) { kv.set('knownRelays', JSON.stringify(relays)); } @@ -142,12 +150,13 @@ export async function initRemoteComms( } const cipher = AES_GCM.create(); - const knownRelays = relays ?? getKnownRelays(kv); + const knownRelays = relays.length > 0 ? relays : getKnownRelays(kv); logger?.log(`relays: ${JSON.stringify(knownRelays)}`); await platformServices.initializeRemoteComms( keySeed, - knownRelays, + { ...options, relays: knownRelays }, remoteMessageHandler, + onRemoteGiveUp, ); /** @@ -226,8 +235,5 @@ export async function initRemoteComms( sendRemoteMessage, issueOcapURL, redeemLocalOcapURL, - stopRemoteComms: platformServices.stopRemoteComms.bind(platformServices), - closeConnection: platformServices.closeConnection.bind(platformServices), - reconnectPeer: platformServices.reconnectPeer.bind(platformServices), }; } diff --git a/packages/ocap-kernel/src/remotes/types.ts b/packages/ocap-kernel/src/remotes/types.ts index edd8ee098..8f2ca4363 100644 --- a/packages/ocap-kernel/src/remotes/types.ts +++ b/packages/ocap-kernel/src/remotes/types.ts @@ -26,9 +26,28 @@ export type RemoteComms = { sendRemoteMessage: SendRemoteMessage; issueOcapURL: (kref: string) => Promise; redeemLocalOcapURL: (ocapURL: string) => Promise; - stopRemoteComms: StopRemoteComms; - closeConnection: (peerId: string) => Promise; - reconnectPeer: (peerId: string, hints?: string[]) => Promise; +}; + +export type OnRemoteGiveUp = (peerId: string) => void; + +/** + * Options for initializing remote communications. + */ +export type RemoteCommsOptions = { + /** + * Array of relay peer IDs/multiaddrs to use for remote communications. + */ + relays?: string[] | undefined; + /** + * Maximum number of reconnection attempts. 0 = infinite (default). + * If not provided, uses DEFAULT_MAX_RETRY_ATTEMPTS. + */ + maxRetryAttempts?: number | undefined; + /** + * Maximum number of messages to queue per peer while reconnecting. + * If not provided, uses the default MAX_QUEUE value. + */ + maxQueue?: number | undefined; }; export type RemoteInfo = { diff --git a/packages/ocap-kernel/src/rpc/kernel-remote/index.test.ts b/packages/ocap-kernel/src/rpc/kernel-remote/index.test.ts index 9d237950d..eb270f96c 100644 --- a/packages/ocap-kernel/src/rpc/kernel-remote/index.test.ts +++ b/packages/ocap-kernel/src/rpc/kernel-remote/index.test.ts @@ -10,7 +10,12 @@ describe('kernel-remote index', () => { expect(kernelRemoteHandlers.remoteDeliver).toBeDefined(); }); - it('should have correct handler structure', () => { + it('should export remoteGiveUp handler', () => { + expect(kernelRemoteHandlers).toHaveProperty('remoteGiveUp'); + expect(kernelRemoteHandlers.remoteGiveUp).toBeDefined(); + }); + + it('should have correct handler structure for remoteDeliver', () => { const handler = kernelRemoteHandlers.remoteDeliver; expect(handler).toHaveProperty('method', 'remoteDeliver'); @@ -20,7 +25,17 @@ describe('kernel-remote index', () => { expect(handler).toHaveProperty('implementation'); }); - it('should have correct hooks configuration', () => { + it('should have correct handler structure for remoteGiveUp', () => { + const handler = kernelRemoteHandlers.remoteGiveUp; + + expect(handler).toHaveProperty('method', 'remoteGiveUp'); + expect(handler).toHaveProperty('params'); + expect(handler).toHaveProperty('result'); + expect(handler).toHaveProperty('hooks'); + expect(handler).toHaveProperty('implementation'); + }); + + it('should have correct hooks configuration for remoteDeliver', () => { const handler = kernelRemoteHandlers.remoteDeliver; expect(handler.hooks).toStrictEqual({ @@ -28,11 +43,25 @@ describe('kernel-remote index', () => { }); }); - it('should have implementation as a function', () => { + it('should have correct hooks configuration for remoteGiveUp', () => { + const handler = kernelRemoteHandlers.remoteGiveUp; + + expect(handler.hooks).toStrictEqual({ + remoteGiveUp: true, + }); + }); + + it('should have implementation as a function for remoteDeliver', () => { const handler = kernelRemoteHandlers.remoteDeliver; expect(typeof handler.implementation).toBe('function'); }); + + it('should have implementation as a function for remoteGiveUp', () => { + const handler = kernelRemoteHandlers.remoteGiveUp; + + expect(typeof handler.implementation).toBe('function'); + }); }); describe('kernelRemoteMethodSpecs', () => { @@ -41,7 +70,12 @@ describe('kernel-remote index', () => { expect(kernelRemoteMethodSpecs.remoteDeliver).toBeDefined(); }); - it('should have correct method spec structure', () => { + it('should export remoteGiveUp method spec', () => { + expect(kernelRemoteMethodSpecs).toHaveProperty('remoteGiveUp'); + expect(kernelRemoteMethodSpecs.remoteGiveUp).toBeDefined(); + }); + + it('should have correct method spec structure for remoteDeliver', () => { const spec = kernelRemoteMethodSpecs.remoteDeliver; expect(spec).toHaveProperty('method', 'remoteDeliver'); @@ -49,12 +83,27 @@ describe('kernel-remote index', () => { expect(spec).toHaveProperty('result'); }); - it('should match the handler method name', () => { + it('should have correct method spec structure for remoteGiveUp', () => { + const spec = kernelRemoteMethodSpecs.remoteGiveUp; + + expect(spec).toHaveProperty('method', 'remoteGiveUp'); + expect(spec).toHaveProperty('params'); + expect(spec).toHaveProperty('result'); + }); + + it('should match the handler method name for remoteDeliver', () => { const handler = kernelRemoteHandlers.remoteDeliver; const spec = kernelRemoteMethodSpecs.remoteDeliver; expect(handler.method).toBe(spec.method); }); + + it('should match the handler method name for remoteGiveUp', () => { + const handler = kernelRemoteHandlers.remoteGiveUp; + const spec = kernelRemoteMethodSpecs.remoteGiveUp; + + expect(handler.method).toBe(spec.method); + }); }); describe('KernelRemoteMethod type', () => { @@ -63,6 +112,12 @@ describe('kernel-remote index', () => { const method: KernelRemoteMethod = 'remoteDeliver'; expect(method).toBe('remoteDeliver'); }); + + it('should include remoteGiveUp method', () => { + // This test verifies that the type is correctly inferred + const method: KernelRemoteMethod = 'remoteGiveUp'; + expect(method).toBe('remoteGiveUp'); + }); }); describe('module consistency', () => { diff --git a/packages/ocap-kernel/src/rpc/kernel-remote/index.ts b/packages/ocap-kernel/src/rpc/kernel-remote/index.ts index e554a1426..1221bafa1 100644 --- a/packages/ocap-kernel/src/rpc/kernel-remote/index.ts +++ b/packages/ocap-kernel/src/rpc/kernel-remote/index.ts @@ -3,17 +3,23 @@ import type { RemoteDeliverSpec, RemoteDeliverHandler, } from './remoteDeliver.ts'; +import { remoteGiveUpSpec, remoteGiveUpHandler } from './remoteGiveUp.ts'; +import type { RemoteGiveUpSpec, RemoteGiveUpHandler } from './remoteGiveUp.ts'; export const kernelRemoteHandlers = { remoteDeliver: remoteDeliverHandler, + remoteGiveUp: remoteGiveUpHandler, } as { remoteDeliver: RemoteDeliverHandler; + remoteGiveUp: RemoteGiveUpHandler; }; export const kernelRemoteMethodSpecs = { remoteDeliver: remoteDeliverSpec, + remoteGiveUp: remoteGiveUpSpec, } as { remoteDeliver: RemoteDeliverSpec; + remoteGiveUp: RemoteGiveUpSpec; }; type Handlers = diff --git a/packages/ocap-kernel/src/rpc/kernel-remote/remoteGiveUp.test.ts b/packages/ocap-kernel/src/rpc/kernel-remote/remoteGiveUp.test.ts new file mode 100644 index 000000000..332703513 --- /dev/null +++ b/packages/ocap-kernel/src/rpc/kernel-remote/remoteGiveUp.test.ts @@ -0,0 +1,119 @@ +import { is } from '@metamask/superstruct'; +import { describe, it, expect, vi } from 'vitest'; + +import type { HandleRemoteGiveUp } from './remoteGiveUp.ts'; +import { remoteGiveUpSpec, remoteGiveUpHandler } from './remoteGiveUp.ts'; + +describe('remoteGiveUp', () => { + describe('remoteGiveUpSpec', () => { + it('should have correct method name', () => { + expect(remoteGiveUpSpec.method).toBe('remoteGiveUp'); + }); + + it('should have correct result type', () => { + expect(is(null, remoteGiveUpSpec.result)).toBe(true); + expect(is('not-null', remoteGiveUpSpec.result)).toBe(false); + expect(is(123, remoteGiveUpSpec.result)).toBe(false); + expect(is(undefined, remoteGiveUpSpec.result)).toBe(false); + }); + + it('should validate params correctly', () => { + const validParams = { + peerId: 'peer-123', + }; + expect(is(validParams, remoteGiveUpSpec.params)).toBe(true); + const invalidParams = { + peerId: 123, + }; + expect(is(invalidParams, remoteGiveUpSpec.params)).toBe(false); + }); + }); + + describe('remoteGiveUpHandler', () => { + it('should have correct result type', () => { + expect(remoteGiveUpHandler.method).toBe('remoteGiveUp'); + expect(remoteGiveUpHandler.params).toBeDefined(); + expect(remoteGiveUpHandler.result).toBeDefined(); + expect(remoteGiveUpHandler.hooks).toStrictEqual({ + remoteGiveUp: true, + }); + }); + + it('should call the remoteGiveUp hook with correct parameters', async () => { + const mockRemoteGiveUp: HandleRemoteGiveUp = vi.fn( + async (_peerId: string) => null, + ); + const hooks = { + remoteGiveUp: mockRemoteGiveUp, + }; + const params = { + peerId: 'peer-123', + }; + const result = await remoteGiveUpHandler.implementation(hooks, params); + expect(mockRemoteGiveUp).toHaveBeenCalledTimes(1); + expect(mockRemoteGiveUp).toHaveBeenCalledWith('peer-123'); + expect(result).toBeNull(); + }); + + it('should return null after calling the hook', async () => { + const mockRemoteGiveUp: HandleRemoteGiveUp = vi.fn( + async (_peerId: string) => null, + ); + const hooks = { + remoteGiveUp: mockRemoteGiveUp, + }; + const params = { + peerId: 'test-peer', + }; + const result = await remoteGiveUpHandler.implementation(hooks, params); + expect(result).toBeNull(); + }); + + it('should propagate errors from the hook', async () => { + const mockRemoteGiveUp: HandleRemoteGiveUp = vi.fn(async () => { + throw new Error('Remote give up failed'); + }); + const hooks = { + remoteGiveUp: mockRemoteGiveUp, + }; + const params = { + peerId: 'test-peer', + }; + await expect( + remoteGiveUpHandler.implementation(hooks, params), + ).rejects.toThrow('Remote give up failed'); + }); + + it('should handle empty string parameters', async () => { + const mockRemoteGiveUp: HandleRemoteGiveUp = vi.fn(async () => null); + const hooks = { + remoteGiveUp: mockRemoteGiveUp, + }; + const params = { + peerId: '', + }; + const result = await remoteGiveUpHandler.implementation(hooks, params); + expect(mockRemoteGiveUp).toHaveBeenCalledWith(''); + expect(result).toBeNull(); + }); + + it('should handle async hook that returns a Promise', async () => { + const mockRemoteGiveUp: HandleRemoteGiveUp = vi.fn( + async (_peerId: string) => { + // Simulate async work + await new Promise((resolve) => setTimeout(resolve, 1)); + return null; + }, + ); + const hooks = { + remoteGiveUp: mockRemoteGiveUp, + }; + const params = { + peerId: 'async-peer', + }; + const result = await remoteGiveUpHandler.implementation(hooks, params); + expect(result).toBeNull(); + expect(mockRemoteGiveUp).toHaveBeenCalledWith('async-peer'); + }); + }); +}); diff --git a/packages/ocap-kernel/src/rpc/kernel-remote/remoteGiveUp.ts b/packages/ocap-kernel/src/rpc/kernel-remote/remoteGiveUp.ts new file mode 100644 index 000000000..27d5cdbc1 --- /dev/null +++ b/packages/ocap-kernel/src/rpc/kernel-remote/remoteGiveUp.ts @@ -0,0 +1,43 @@ +import type { MethodSpec, Handler } from '@metamask/kernel-rpc-methods'; +import { object, string, literal } from '@metamask/superstruct'; +import type { Infer } from '@metamask/superstruct'; + +const paramsStruct = object({ + peerId: string(), +}); + +type Params = Infer; + +export type RemoteGiveUpSpec = MethodSpec< + 'remoteGiveUp', + { peerId: string }, + null +>; + +export const remoteGiveUpSpec: RemoteGiveUpSpec = { + method: 'remoteGiveUp', + params: paramsStruct, + result: literal(null), +}; + +export type HandleRemoteGiveUp = (peerId: string) => Promise; + +type RemoteGiveUpHooks = { + remoteGiveUp: HandleRemoteGiveUp; +}; + +export type RemoteGiveUpHandler = Handler< + 'remoteGiveUp', + Params, + Promise, + RemoteGiveUpHooks +>; + +export const remoteGiveUpHandler: RemoteGiveUpHandler = { + ...remoteGiveUpSpec, + hooks: { remoteGiveUp: true }, + implementation: async ({ remoteGiveUp }, params) => { + await remoteGiveUp(params.peerId); + return null; + }, +}; diff --git a/packages/ocap-kernel/src/rpc/platform-services/initializeRemoteComms.test.ts b/packages/ocap-kernel/src/rpc/platform-services/initializeRemoteComms.test.ts index b5f482242..6d718c92f 100644 --- a/packages/ocap-kernel/src/rpc/platform-services/initializeRemoteComms.test.ts +++ b/packages/ocap-kernel/src/rpc/platform-services/initializeRemoteComms.test.ts @@ -6,6 +6,7 @@ import { initializeRemoteCommsSpec, initializeRemoteCommsHandler, } from './initializeRemoteComms.ts'; +import type { RemoteCommsOptions } from '../../remotes/types.ts'; describe('initializeRemoteComms', () => { describe('initializeRemoteCommsSpec', () => { @@ -25,7 +26,7 @@ describe('initializeRemoteComms', () => { it('should accept valid params', () => { const validParams = { keySeed: '0x1234567890abcdef', - knownRelays: [ + relays: [ '/dns4/relay1.example/tcp/443/wss/p2p/relay1', '/dns4/relay2.example/tcp/443/wss/p2p/relay2', ], @@ -34,26 +35,44 @@ describe('initializeRemoteComms', () => { expect(is(validParams, initializeRemoteCommsSpec.params)).toBe(true); }); - it('should accept params with empty knownRelays array', () => { + it('should accept params with empty relays array', () => { const validParams = { keySeed: '0xabcdef1234567890', - knownRelays: [], + relays: [], }; expect(is(validParams, initializeRemoteCommsSpec.params)).toBe(true); }); - it('should reject params with missing keySeed', () => { - const invalidParams = { - knownRelays: ['/dns4/relay.example/tcp/443/wss/p2p/relay'], + it('should accept params with only keySeed', () => { + const validParams = { + keySeed: '0x1234567890abcdef', }; - expect(is(invalidParams, initializeRemoteCommsSpec.params)).toBe(false); + expect(is(validParams, initializeRemoteCommsSpec.params)).toBe(true); }); - it('should reject params with missing knownRelays', () => { - const invalidParams = { + it('should accept params with maxRetryAttempts', () => { + const validParams = { + keySeed: '0x1234567890abcdef', + maxRetryAttempts: 5, + }; + + expect(is(validParams, initializeRemoteCommsSpec.params)).toBe(true); + }); + + it('should accept params with maxQueue', () => { + const validParams = { keySeed: '0x1234567890abcdef', + maxQueue: 100, + }; + + expect(is(validParams, initializeRemoteCommsSpec.params)).toBe(true); + }); + + it('should reject params with missing keySeed', () => { + const invalidParams = { + relays: ['/dns4/relay.example/tcp/443/wss/p2p/relay'], }; expect(is(invalidParams, initializeRemoteCommsSpec.params)).toBe(false); @@ -62,7 +81,7 @@ describe('initializeRemoteComms', () => { it('should reject params with non-string keySeed', () => { const invalidParams = { keySeed: 123, - knownRelays: [], + relays: [], }; expect(is(invalidParams, initializeRemoteCommsSpec.params)).toBe(false); @@ -71,16 +90,16 @@ describe('initializeRemoteComms', () => { it('should reject params with non-array knownRelays', () => { const invalidParams = { keySeed: '0x1234567890abcdef', - knownRelays: 'not-an-array', + relays: 'not-an-array', }; expect(is(invalidParams, initializeRemoteCommsSpec.params)).toBe(false); }); - it('should reject params with non-string elements in knownRelays', () => { + it('should reject params with non-string elements in relays', () => { const invalidParams = { keySeed: '0x1234567890abcdef', - knownRelays: [ + relays: [ '/dns4/relay1.example/tcp/443/wss/p2p/relay1', 123, // non-string element ], @@ -89,10 +108,28 @@ describe('initializeRemoteComms', () => { expect(is(invalidParams, initializeRemoteCommsSpec.params)).toBe(false); }); + it('should reject params with non-number maxRetryAttempts', () => { + const invalidParams = { + keySeed: '0x1234567890abcdef', + maxRetryAttempts: 'not-a-number', + }; + + expect(is(invalidParams, initializeRemoteCommsSpec.params)).toBe(false); + }); + + it('should reject params with non-number maxQueue', () => { + const invalidParams = { + keySeed: '0x1234567890abcdef', + maxQueue: 'not-a-number', + }; + + expect(is(invalidParams, initializeRemoteCommsSpec.params)).toBe(false); + }); + it('should reject params with extra fields', () => { const invalidParams = { keySeed: '0x1234567890abcdef', - knownRelays: [], + relays: [], extra: 'field', }; @@ -116,16 +153,16 @@ describe('initializeRemoteComms', () => { it('should accept empty string keySeed', () => { const validParams = { keySeed: '', - knownRelays: [], + relays: [], }; expect(is(validParams, initializeRemoteCommsSpec.params)).toBe(true); }); - it('should accept empty strings in knownRelays', () => { + it('should accept empty strings in relays', () => { const validParams = { keySeed: '0x1234567890abcdef', - knownRelays: ['', ''], + relays: ['', ''], }; expect(is(validParams, initializeRemoteCommsSpec.params)).toBe(true); @@ -134,7 +171,7 @@ describe('initializeRemoteComms', () => { it('should accept unicode characters', () => { const validParams = { keySeed: '🔑seed🔑', - knownRelays: ['🌐relay🌐', '🔗connection🔗'], + relays: ['🌐relay🌐', '🔗connection🔗'], }; expect(is(validParams, initializeRemoteCommsSpec.params)).toBe(true); @@ -144,7 +181,7 @@ describe('initializeRemoteComms', () => { const longString = 'a'.repeat(10000); const validParams = { keySeed: longString, - knownRelays: [longString, longString], + relays: [longString, longString], }; expect(is(validParams, initializeRemoteCommsSpec.params)).toBe(true); @@ -158,7 +195,7 @@ describe('initializeRemoteComms', () => { const validParams = { keySeed: '0x1234567890abcdef', - knownRelays: manyRelays, + relays: manyRelays, }; expect(is(validParams, initializeRemoteCommsSpec.params)).toBe(true); @@ -188,7 +225,7 @@ describe('initializeRemoteComms', () => { const params = { keySeed: '0x1234567890abcdef', - knownRelays: [ + relays: [ '/dns4/relay1.example/tcp/443/wss/p2p/relay1', '/dns4/relay2.example/tcp/443/wss/p2p/relay2', ], @@ -202,10 +239,12 @@ describe('initializeRemoteComms', () => { expect(mockInitializeRemoteComms).toHaveBeenCalledTimes(1); expect(mockInitializeRemoteComms).toHaveBeenCalledWith( '0x1234567890abcdef', - [ - '/dns4/relay1.example/tcp/443/wss/p2p/relay1', - '/dns4/relay2.example/tcp/443/wss/p2p/relay2', - ], + { + relays: [ + '/dns4/relay1.example/tcp/443/wss/p2p/relay1', + '/dns4/relay2.example/tcp/443/wss/p2p/relay2', + ], + }, ); expect(result).toBeNull(); }); @@ -221,7 +260,7 @@ describe('initializeRemoteComms', () => { const params = { keySeed: 'test-seed', - knownRelays: ['test-relay'], + relays: ['test-relay'], }; const result = await initializeRemoteCommsHandler.implementation( @@ -245,7 +284,7 @@ describe('initializeRemoteComms', () => { const params = { keySeed: 'failing-seed', - knownRelays: ['failing-relay'], + relays: ['failing-relay'], }; await expect( @@ -264,15 +303,14 @@ describe('initializeRemoteComms', () => { const params = { keySeed: '0xemptyrelays', - knownRelays: [], + relays: [], }; await initializeRemoteCommsHandler.implementation(hooks, params); - expect(mockInitializeRemoteComms).toHaveBeenCalledWith( - '0xemptyrelays', - [], - ); + expect(mockInitializeRemoteComms).toHaveBeenCalledWith('0xemptyrelays', { + relays: [], + }); }); it('should handle empty string parameters', async () => { @@ -286,12 +324,14 @@ describe('initializeRemoteComms', () => { const params = { keySeed: '', - knownRelays: [''], + relays: [''], }; await initializeRemoteCommsHandler.implementation(hooks, params); - expect(mockInitializeRemoteComms).toHaveBeenCalledWith('', ['']); + expect(mockInitializeRemoteComms).toHaveBeenCalledWith('', { + relays: [''], + }); }); it('should handle unicode characters in parameters', async () => { @@ -305,14 +345,14 @@ describe('initializeRemoteComms', () => { const params = { keySeed: '🔑unicode-seed🔑', - knownRelays: ['🌐unicode-relay🌐'], + relays: ['🌐unicode-relay🌐'], }; await initializeRemoteCommsHandler.implementation(hooks, params); expect(mockInitializeRemoteComms).toHaveBeenCalledWith( '🔑unicode-seed🔑', - ['🌐unicode-relay🌐'], + { relays: ['🌐unicode-relay🌐'] }, ); }); @@ -331,7 +371,7 @@ describe('initializeRemoteComms', () => { const params = { keySeed: 'async-seed', - knownRelays: ['async-relay'], + relays: ['async-relay'], }; const result = await initializeRemoteCommsHandler.implementation( @@ -362,16 +402,16 @@ describe('initializeRemoteComms', () => { const params = { keySeed, - knownRelays: ['test-relay'], + relays: ['test-relay'], }; await expect( initializeRemoteCommsHandler.implementation(hooks, params), ).rejects.toThrow(error); - expect(mockInitializeRemoteComms).toHaveBeenCalledWith(keySeed, [ - 'test-relay', - ]); + expect(mockInitializeRemoteComms).toHaveBeenCalledWith(keySeed, { + relays: ['test-relay'], + }); }, ); @@ -391,21 +431,20 @@ describe('initializeRemoteComms', () => { const params = { keySeed: '0xmanyrelays', - knownRelays: manyRelays, + relays: manyRelays, }; await initializeRemoteCommsHandler.implementation(hooks, params); - expect(mockInitializeRemoteComms).toHaveBeenCalledWith( - '0xmanyrelays', - manyRelays, - ); + expect(mockInitializeRemoteComms).toHaveBeenCalledWith('0xmanyrelays', { + relays: manyRelays, + }); }); it('should handle complex initialization scenarios', async () => { let initializationSteps = 0; const mockInitializeRemoteComms: InitializeRemoteComms = vi.fn( - async (keySeed: string, knownRelays: string[]) => { + async (keySeed: string, options: RemoteCommsOptions) => { // Simulate complex initialization initializationSteps += 1; // Step 1: Parse key seed await new Promise((resolve) => setTimeout(resolve, 1)); @@ -420,7 +459,7 @@ describe('initializeRemoteComms', () => { throw new Error('Invalid key seed'); } - if (knownRelays.length === 0) { + if (!options.relays || options.relays.length === 0) { throw new Error('No relays provided'); } @@ -434,7 +473,7 @@ describe('initializeRemoteComms', () => { const params = { keySeed: '0xcomplexinitialization', - knownRelays: ['/dns4/complex.relay/tcp/443/wss/p2p/complex'], + relays: ['/dns4/complex.relay/tcp/443/wss/p2p/complex'], }; const result = await initializeRemoteCommsHandler.implementation( @@ -445,5 +484,183 @@ describe('initializeRemoteComms', () => { expect(result).toBeNull(); expect(initializationSteps).toBe(3); }); + + it('should pass maxRetryAttempts to hook when provided', async () => { + const mockInitializeRemoteComms: InitializeRemoteComms = vi.fn( + async () => null, + ); + + const hooks = { + initializeRemoteComms: mockInitializeRemoteComms, + }; + + const params = { + keySeed: '0xtestseed', + maxRetryAttempts: 5, + }; + + await initializeRemoteCommsHandler.implementation(hooks, params); + + expect(mockInitializeRemoteComms).toHaveBeenCalledWith('0xtestseed', { + maxRetryAttempts: 5, + }); + }); + + it('should pass maxQueue to hook when provided', async () => { + const mockInitializeRemoteComms: InitializeRemoteComms = vi.fn( + async () => null, + ); + + const hooks = { + initializeRemoteComms: mockInitializeRemoteComms, + }; + + const params = { + keySeed: '0xtestseed', + maxQueue: 100, + }; + + await initializeRemoteCommsHandler.implementation(hooks, params); + + expect(mockInitializeRemoteComms).toHaveBeenCalledWith('0xtestseed', { + maxQueue: 100, + }); + }); + + it('should pass all options when all are provided', async () => { + const mockInitializeRemoteComms: InitializeRemoteComms = vi.fn( + async () => null, + ); + + const hooks = { + initializeRemoteComms: mockInitializeRemoteComms, + }; + + const params = { + keySeed: '0xtestseed', + relays: ['/dns4/relay.example/tcp/443/wss/p2p/relay'], + maxRetryAttempts: 5, + maxQueue: 100, + }; + + await initializeRemoteCommsHandler.implementation(hooks, params); + + expect(mockInitializeRemoteComms).toHaveBeenCalledWith('0xtestseed', { + relays: ['/dns4/relay.example/tcp/443/wss/p2p/relay'], + maxRetryAttempts: 5, + maxQueue: 100, + }); + }); + + it('should pass empty options when only keySeed is provided', async () => { + const mockInitializeRemoteComms: InitializeRemoteComms = vi.fn( + async () => null, + ); + + const hooks = { + initializeRemoteComms: mockInitializeRemoteComms, + }; + + const params = { + keySeed: '0xtestseed', + }; + + await initializeRemoteCommsHandler.implementation(hooks, params); + + expect(mockInitializeRemoteComms).toHaveBeenCalledWith('0xtestseed', {}); + }); + + it('should not include undefined optional params in options', async () => { + const mockInitializeRemoteComms: InitializeRemoteComms = vi.fn( + async (_keySeed: string, options: RemoteCommsOptions) => { + // Verify that undefined params are not included + expect(options).not.toHaveProperty('relays'); + expect(options).not.toHaveProperty('maxRetryAttempts'); + expect(options).not.toHaveProperty('maxQueue'); + return null; + }, + ); + + const hooks = { + initializeRemoteComms: mockInitializeRemoteComms, + }; + + const params = { + keySeed: '0xtestseed', + }; + + await initializeRemoteCommsHandler.implementation(hooks, params); + }); + + it('should accept params with all optional fields', () => { + const validParams = { + keySeed: '0x1234567890abcdef', + relays: ['/dns4/relay.example/tcp/443/wss/p2p/relay'], + maxRetryAttempts: 5, + maxQueue: 100, + }; + + expect(is(validParams, initializeRemoteCommsSpec.params)).toBe(true); + }); + + it('should accept params with maxRetryAttempts set to zero', () => { + const validParams = { + keySeed: '0x1234567890abcdef', + maxRetryAttempts: 0, + }; + + expect(is(validParams, initializeRemoteCommsSpec.params)).toBe(true); + }); + + it('should accept params with maxQueue set to zero', () => { + const validParams = { + keySeed: '0x1234567890abcdef', + maxQueue: 0, + }; + + expect(is(validParams, initializeRemoteCommsSpec.params)).toBe(true); + }); + + it('should pass maxRetryAttempts set to zero to hook', async () => { + const mockInitializeRemoteComms: InitializeRemoteComms = vi.fn( + async () => null, + ); + + const hooks = { + initializeRemoteComms: mockInitializeRemoteComms, + }; + + const params = { + keySeed: '0xtestseed', + maxRetryAttempts: 0, + }; + + await initializeRemoteCommsHandler.implementation(hooks, params); + + expect(mockInitializeRemoteComms).toHaveBeenCalledWith('0xtestseed', { + maxRetryAttempts: 0, + }); + }); + + it('should pass maxQueue set to zero to hook', async () => { + const mockInitializeRemoteComms: InitializeRemoteComms = vi.fn( + async () => null, + ); + + const hooks = { + initializeRemoteComms: mockInitializeRemoteComms, + }; + + const params = { + keySeed: '0xtestseed', + maxQueue: 0, + }; + + await initializeRemoteCommsHandler.implementation(hooks, params); + + expect(mockInitializeRemoteComms).toHaveBeenCalledWith('0xtestseed', { + maxQueue: 0, + }); + }); }); }); diff --git a/packages/ocap-kernel/src/rpc/platform-services/initializeRemoteComms.ts b/packages/ocap-kernel/src/rpc/platform-services/initializeRemoteComms.ts index c90214081..fc882c414 100644 --- a/packages/ocap-kernel/src/rpc/platform-services/initializeRemoteComms.ts +++ b/packages/ocap-kernel/src/rpc/platform-services/initializeRemoteComms.ts @@ -1,15 +1,28 @@ import type { MethodSpec, Handler } from '@metamask/kernel-rpc-methods'; -import { array, object, literal, string } from '@metamask/superstruct'; -import type { Infer } from '@metamask/superstruct'; +import { + array, + object, + literal, + string, + number, + optional, +} from '@metamask/superstruct'; + +import type { RemoteCommsOptions } from '../../remotes/types.ts'; const initializeRemoteCommsParamsStruct = object({ keySeed: string(), - knownRelays: array(string()), + relays: optional(array(string())), + maxRetryAttempts: optional(number()), + maxQueue: optional(number()), }); -type InitializeRemoteCommsParams = Infer< - typeof initializeRemoteCommsParamsStruct ->; +type InitializeRemoteCommsParams = { + keySeed: string; + relays?: string[]; + maxRetryAttempts?: number; + maxQueue?: number; +}; export type InitializeRemoteCommsSpec = MethodSpec< 'initializeRemoteComms', @@ -21,11 +34,11 @@ export const initializeRemoteCommsSpec: InitializeRemoteCommsSpec = { method: 'initializeRemoteComms', params: initializeRemoteCommsParamsStruct, result: literal(null), -}; +} as InitializeRemoteCommsSpec; export type InitializeRemoteComms = ( keySeed: string, - knownRelays: string[], + options: RemoteCommsOptions, ) => Promise; type InitializeRemoteCommsHooks = { @@ -43,6 +56,16 @@ export const initializeRemoteCommsHandler: InitializeRemoteCommsHandler = { ...initializeRemoteCommsSpec, hooks: { initializeRemoteComms: true }, implementation: async ({ initializeRemoteComms }, params) => { - return await initializeRemoteComms(params.keySeed, params.knownRelays); + const options: RemoteCommsOptions = {}; + if (params.relays !== undefined) { + options.relays = params.relays; + } + if (params.maxRetryAttempts !== undefined) { + options.maxRetryAttempts = params.maxRetryAttempts; + } + if (params.maxQueue !== undefined) { + options.maxQueue = params.maxQueue; + } + return await initializeRemoteComms(params.keySeed, options); }, }; diff --git a/packages/ocap-kernel/src/store/methods/promise.ts b/packages/ocap-kernel/src/store/methods/promise.ts index ffca4ce65..48044a887 100644 --- a/packages/ocap-kernel/src/store/methods/promise.ts +++ b/packages/ocap-kernel/src/store/methods/promise.ts @@ -9,7 +9,6 @@ import type { Message, PromiseState, RunQueueItemSend, - VatId, EndpointId, } from '../../types.ts'; import { insistEndpointId } from '../../types.ts'; @@ -170,7 +169,7 @@ export function getPromiseMethods(ctx: StoreContext) { ctx.kv.set(`${kpid}.value`, JSON.stringify(value)); ctx.kv.delete(`${kpid}.decider`); ctx.kv.delete(`${kpid}.subscribers`); - // Drop the baseline “decider” refcount now that the promise is settled. + // Drop the baseline "decider" refcount now that the promise is settled. decrementRefCount(kpid, 'resolve|decider'); queue.delete(); } @@ -203,14 +202,15 @@ export function getPromiseMethods(ctx: StoreContext) { } } } + /** - * Generator that yield the promises decided by a given vat. + * Generator that yield the promises decided by a given endpoint (vat or remote). * - * @param decider - The vat ID of the vat of interest. + * @param decider - The endpoint ID (vat ID or remote ID) of interest. * - * @yields the kpids of all the promises decided by `decider`. + * @yields the kpids of all the unresolved promises decided by `decider`. */ - function* getPromisesByDecider(decider: VatId): Generator { + function* getPromisesByDecider(decider: EndpointId): Generator { const basePrefix = `cle.${decider}.`; for (const key of getPrefixedKeys(`${basePrefix}p`)) { const kpid = ctx.kv.getRequired(key); diff --git a/packages/ocap-kernel/src/types.ts b/packages/ocap-kernel/src/types.ts index 6a59ce1c5..5ed0d86f7 100644 --- a/packages/ocap-kernel/src/types.ts +++ b/packages/ocap-kernel/src/types.ts @@ -33,6 +33,8 @@ import type { RemoteMessageHandler, SendRemoteMessage, StopRemoteComms, + OnRemoteGiveUp, + RemoteCommsOptions, } from './remotes/types.ts'; import { Fail } from './utils/assert.ts'; @@ -316,16 +318,21 @@ export type PlatformServices = { * Initialize network communications. * * @param keySeed - The seed for generating this kernel's secret key. - * @param knownRelays - Array of the peerIDs of relay nodes that can be used to listen for incoming + * @param options - Options for remote communications initialization. + * @param options.relays - Array of the peerIDs of relay nodes that can be used to listen for incoming * connections from other kernels. + * @param options.maxRetryAttempts - Maximum number of reconnection attempts. 0 = infinite (default). + * @param options.maxQueue - Maximum number of messages to queue per peer while reconnecting (default: 200). * @param remoteMessageHandler - A handler function to receive remote messages. + * @param onRemoteGiveUp - Optional callback to be called when we give up on a remote. * @returns A promise that resolves once network access has been established * or rejects if there is some problem doing so. */ initializeRemoteComms: ( keySeed: string, - knownRelays: string[], + options: RemoteCommsOptions, remoteMessageHandler: RemoteMessageHandler, + onRemoteGiveUp?: OnRemoteGiveUp, ) => Promise; /** * Stop network communications. diff --git a/packages/ocap-kernel/test/remotes-mocks.ts b/packages/ocap-kernel/test/remotes-mocks.ts index bcf38b83f..564917e10 100644 --- a/packages/ocap-kernel/test/remotes-mocks.ts +++ b/packages/ocap-kernel/test/remotes-mocks.ts @@ -88,9 +88,6 @@ export class MockRemotesFactory { .fn() .mockResolvedValue(`ocap:abc123@${this.config.peerId}`), redeemLocalOcapURL: vi.fn().mockResolvedValue('ko123'), - stopRemoteComms: vi.fn().mockResolvedValue(undefined), - closeConnection: vi.fn().mockResolvedValue(undefined), - reconnectPeer: vi.fn().mockResolvedValue(undefined), ...overrides, }; } diff --git a/vitest.config.ts b/vitest.config.ts index a0624ff29..897319a63 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -86,10 +86,10 @@ export default defineConfig({ lines: 94.15, }, 'packages/kernel-browser-runtime/**': { - statements: 84.21, - functions: 90.62, - branches: 93.33, - lines: 84.21, + statements: 87.37, + functions: 93.93, + branches: 96.94, + lines: 87.37, }, 'packages/kernel-errors/**': { statements: 100, @@ -106,7 +106,7 @@ export default defineConfig({ 'packages/kernel-platforms/**': { statements: 99.38, functions: 100, - branches: 96.25, + branches: 96.2, lines: 99.38, }, 'packages/kernel-rpc-methods/**': { @@ -130,7 +130,7 @@ export default defineConfig({ 'packages/kernel-ui/**': { statements: 97.57, functions: 97.29, - branches: 93.26, + branches: 93.25, lines: 97.57, }, 'packages/kernel-utils/**': { @@ -146,10 +146,10 @@ export default defineConfig({ lines: 100, }, 'packages/nodejs/**': { - statements: 87.76, - functions: 88.23, - branches: 95.34, - lines: 87.76, + statements: 90.27, + functions: 94.11, + branches: 95.65, + lines: 90.27, }, 'packages/nodejs-test-workers/**': { statements: 22.22, @@ -158,10 +158,10 @@ export default defineConfig({ lines: 22.22, }, 'packages/ocap-kernel/**': { - statements: 96.6, - functions: 98.52, - branches: 97.54, - lines: 96.6, + statements: 96.63, + functions: 98.53, + branches: 97.8, + lines: 96.63, }, 'packages/omnium-gatherum/**': { statements: 5.67,