From 8ea777a556f7402b14f1d4449f623ec25669cecb Mon Sep 17 00:00:00 2001 From: Chip Morningstar Date: Tue, 4 Aug 2026 15:12:11 -0700 Subject: [PATCH 01/24] fix(ocap-kernel): honor the run-queue length cache's invalid sentinel `runQueueLengthCache` uses a negative value to mean "unknown, re-read from the DB", but enqueueRun/dequeueRun adjusted it arithmetically without materializing it first. An enqueue while the cache was -1 (its value at daemon startup) produced 0 for a queue that actually held an item, and since 0 isn't negative it was never re-read: the run loop then saw an empty queue, went to sleep, and stranded the queued messages forever, with no error and no log. Also wake the run loop on any non-empty queue rather than only on the empty->1 transition, so a drifted count cannot silently lose the wakeup. Co-Authored-By: Claude Opus 4.7 --- packages/ocap-kernel/src/KernelQueue.ts | 6 +++- .../src/store/methods/queue.test.ts | 35 +++++++++++++++++++ .../ocap-kernel/src/store/methods/queue.ts | 8 +++++ 3 files changed, 48 insertions(+), 1 deletion(-) diff --git a/packages/ocap-kernel/src/KernelQueue.ts b/packages/ocap-kernel/src/KernelQueue.ts index f0cba6d132..a844cfacba 100644 --- a/packages/ocap-kernel/src/KernelQueue.ts +++ b/packages/ocap-kernel/src/KernelQueue.ts @@ -183,7 +183,11 @@ export class KernelQueue { */ #enqueueRun(item: RunQueueItem): void { this.#kernelStore.enqueueRun(item); - if (this.#kernelStore.runQueueLength() === 1 && this.#wakeUpTheRunQueue) { + // Wake on any non-empty queue rather than only on the empty->1 + // transition. A sleeping run loop plus a non-empty queue is a + // permanent wedge, so err towards a spurious wake: the resolver is + // cleared as it fires, and the loop re-checks the queue on waking. + if (this.#kernelStore.runQueueLength() > 0 && this.#wakeUpTheRunQueue) { const wakeUpTheRunQueue = this.#wakeUpTheRunQueue; this.#wakeUpTheRunQueue = null; wakeUpTheRunQueue(); diff --git a/packages/ocap-kernel/src/store/methods/queue.test.ts b/packages/ocap-kernel/src/store/methods/queue.test.ts index d44a3dffb8..39c1ed623c 100644 --- a/packages/ocap-kernel/src/store/methods/queue.test.ts +++ b/packages/ocap-kernel/src/store/methods/queue.test.ts @@ -117,6 +117,26 @@ describe('queue store methods', () => { expect(mockRunQueue.enqueue).toHaveBeenNthCalledWith(1, message1); expect(mockRunQueue.enqueue).toHaveBeenNthCalledWith(2, message2); }); + + it('resolves an invalidated cache from the database before incrementing', () => { + // A negative cache means "length unknown, re-read from the DB". + // Incrementing it blindly would yield 0 for a queue that already + // holds an item, and since 0 is not negative the stale value would + // never be re-read — stranding queued items and losing the run + // loop's wakeup. + const message: RunQueueItem = { + type: 'message', + data: { some: 'data' }, + } as unknown as RunQueueItem; + context.runQueueLengthCache = -1; + mockKV.set('queue.run.head', '38'); + mockKV.set('queue.run.tail', '37'); + + queueMethods.enqueueRun(message); + + expect(context.runQueueLengthCache).toBe(2); + expect(queueMethods.runQueueLength()).toBe(2); + }); }); describe('dequeueRun', () => { @@ -172,6 +192,21 @@ describe('queue store methods', () => { expect(queueMethods.dequeueRun()).toBeUndefined(); expect(context.runQueueLengthCache).toBe(0); }); + + it('resolves an invalidated cache from the database before decrementing', () => { + const message: RunQueueItem = { + type: 'message', + data: { some: 'data' }, + } as unknown as RunQueueItem; + mockRunQueue.dequeue.mockReturnValue(message); + context.runQueueLengthCache = -1; + mockKV.set('queue.run.head', '39'); + mockKV.set('queue.run.tail', '37'); + + expect(queueMethods.dequeueRun()).toStrictEqual(message); + + expect(context.runQueueLengthCache).toBe(1); + }); }); describe('runQueueLength', () => { diff --git a/packages/ocap-kernel/src/store/methods/queue.ts b/packages/ocap-kernel/src/store/methods/queue.ts index 54693580e8..01ed054822 100644 --- a/packages/ocap-kernel/src/store/methods/queue.ts +++ b/packages/ocap-kernel/src/store/methods/queue.ts @@ -33,6 +33,11 @@ export function getQueueMethods(ctx: StoreContext) { * @param message - The message to enqueue. */ function enqueueRun(message: RunQueueItem): void { + // Materialize the cache from the database before adjusting it. A + // negative cache means "unknown"; incrementing it blindly would turn + // that sentinel into a concrete (and wrong) count, and since the + // result is no longer negative it would never be re-read. + runQueueLength(); ctx.runQueueLengthCache += 1; ctx.runQueue.enqueue(message); } @@ -44,6 +49,9 @@ export function getQueueMethods(ctx: StoreContext) { * empty. */ function dequeueRun(): RunQueueItem | undefined { + // Materialize the cache before adjusting it, for the same reason as + // in `enqueueRun`. + runQueueLength(); ctx.runQueueLengthCache -= 1; return ctx.runQueue.dequeue() as RunQueueItem | undefined; } From 0ad94379cf78b6855360af498317f9fa02593779 Mon Sep 17 00:00:00 2001 From: Chip Morningstar Date: Tue, 4 Aug 2026 15:12:18 -0700 Subject: [PATCH 02/24] feat(ocap-kernel): anonymous kernel-hosted objects Adds registerAnonymousKernelObject/releaseAnonymousKernelObject: a kref is allocated and entered in the by-kref routing table, but deliberately not in the service-name index, so the object has no name in the global service namespace and cannot be requested via a cluster config's `services` list. Authority comes from holding the reference. Needed for IOListener.accept(), where each accepted connection is a per-session object that should be reachable only by reference. Returned krefs are handed to kslot() so a kernel service method can return one; krefOf has no allocation path of its own. Co-Authored-By: Claude Opus 4.7 --- .../src/KernelServiceManager.test.ts | 113 +++++++++++++++++- .../ocap-kernel/src/KernelServiceManager.ts | 59 +++++++++ 2 files changed, 171 insertions(+), 1 deletion(-) diff --git a/packages/ocap-kernel/src/KernelServiceManager.test.ts b/packages/ocap-kernel/src/KernelServiceManager.test.ts index 4d82d8af15..3153297c3d 100644 --- a/packages/ocap-kernel/src/KernelServiceManager.test.ts +++ b/packages/ocap-kernel/src/KernelServiceManager.test.ts @@ -4,7 +4,7 @@ import { describe, it, expect, beforeEach, vi } from 'vitest'; import type { KernelQueue } from './KernelQueue.ts'; import { KernelServiceManager } from './KernelServiceManager.ts'; -import { kser, makeKernelError } from './liveslots/kernel-marshal.ts'; +import { kser, kslot, makeKernelError } from './liveslots/kernel-marshal.ts'; import { makeKernelStore } from './store/index.ts'; import type { KernelMessage } from './types.ts'; import { makeMapKernelDatabase } from '../test/storage.ts'; @@ -537,4 +537,115 @@ describe('KernelServiceManager', () => { ]); }); }); + + describe('registerAnonymousKernelObject', () => { + it('hosts the object for routing without naming it', () => { + const kref = serviceManager.registerAnonymousKernelObject( + { ping: () => 'pong' }, + 'io-connection', + ); + + expect(serviceManager.isKernelService(kref)).toBe(true); + expect(kernelStore.getOwner(kref)).toBe('kernel'); + expect(kernelStore.pinObject).toHaveBeenCalledWith(kref); + // The whole point: absent from the global name namespace, so no + // string can be used to ask for it. + expect(serviceManager.getKernelService('io-connection')).toBeUndefined(); + }); + + it('allows the same label for distinct objects', () => { + const first = serviceManager.registerAnonymousKernelObject( + { which: () => 'first' }, + 'io-connection', + ); + const second = serviceManager.registerAnonymousKernelObject( + { which: () => 'second' }, + 'io-connection', + ); + + expect(second).not.toBe(first); + expect(serviceManager.isKernelService(first)).toBe(true); + expect(serviceManager.isKernelService(second)).toBe(true); + }); + + it('round-trips through kslot/kser so a service method can return one', () => { + // This is what makes `accept()` possible: `invokeKernelService` + // passes a method's return value through `kser`, whose val-to-slot + // step only accepts standins minted by `kslot`. + const kref = serviceManager.registerAnonymousKernelObject( + { ping: () => 'pong' }, + 'io-connection', + ); + + expect(kser(kslot(kref))).toStrictEqual({ + body: expect.any(String), + slots: [kref], + }); + }); + + it('delivers messages to the hosted object', async () => { + const { method: ping, calls } = makeTrackableMethod(() => 'pong'); + const kref = serviceManager.registerAnonymousKernelObject( + { ping }, + 'io-connection', + ); + + serviceManager.invokeKernelService(kref, { + methargs: kser(['ping', ['hello']]), + result: 'kp200', + }); + await delay(); + + expect(calls).toStrictEqual([['hello']]); + expect(mockKernelQueue.resolvePromises).toHaveBeenCalledWith('kernel', [ + ['kp200', false, kser('pong')], + ]); + }); + }); + + describe('releaseAnonymousKernelObject', () => { + it('removes the object from routing and unpins it', () => { + const kref = serviceManager.registerAnonymousKernelObject( + { ping: () => 'pong' }, + 'io-connection', + ); + + serviceManager.releaseAnonymousKernelObject(kref); + + expect(serviceManager.isKernelService(kref)).toBe(false); + expect(serviceManager.getKernelServiceByKref(kref)).toBeUndefined(); + expect(kernelStore.isObjectPinned(kref)).toBe(false); + }); + + it('leaves other hosted objects alone', () => { + const kept = serviceManager.registerAnonymousKernelObject( + { which: () => 'kept' }, + 'io-connection', + ); + const dropped = serviceManager.registerAnonymousKernelObject( + { which: () => 'dropped' }, + 'io-connection', + ); + + serviceManager.releaseAnonymousKernelObject(dropped); + + expect(serviceManager.isKernelService(kept)).toBe(true); + expect(serviceManager.isKernelService(dropped)).toBe(false); + }); + + it('is idempotent and tolerates an unregistered kref', () => { + const kref = serviceManager.registerAnonymousKernelObject( + { ping: () => 'pong' }, + 'io-connection', + ); + + serviceManager.releaseAnonymousKernelObject(kref); + expect(() => + serviceManager.releaseAnonymousKernelObject(kref), + ).not.toThrow(); + expect(() => + serviceManager.releaseAnonymousKernelObject('ko9999'), + ).not.toThrow(); + }); + }); }); diff --git a/packages/ocap-kernel/src/KernelServiceManager.ts b/packages/ocap-kernel/src/KernelServiceManager.ts index c907f110b7..d55aeb2957 100644 --- a/packages/ocap-kernel/src/KernelServiceManager.ts +++ b/packages/ocap-kernel/src/KernelServiceManager.ts @@ -8,6 +8,14 @@ import type { KRef, KernelMessage } from './types.ts'; import { assert } from './utils/assert.ts'; export type KernelService = { + /** + * The service's name. For services registered with + * `registerKernelServiceObject` this is the key vats name in their + * cluster config's `services` list, and it is unique. For objects + * registered with `registerAnonymousKernelObject` it is only a + * diagnostic label: those objects are deliberately absent from the + * name index and need not be unique. + */ name: string; kref: KRef; service: object; @@ -103,6 +111,57 @@ export class KernelServiceManager { this.#kernelStore.deleteKernelServiceKref(name); } + /** + * Register a kernel-hosted object reachable *only* by reference. + * + * Unlike `registerKernelServiceObject`, this enters the object in the + * by-kref routing table but deliberately not in the name index, so it + * has no name in the global service namespace and cannot be requested + * via a cluster config's `services` list. The only way to obtain one + * is to be handed the reference, which is what makes it suitable for + * per-session objects such as an accepted IO connection: authority is + * conveyed by an unforgeable reference rather than by a string that + * anything able to name it could use. + * + * The returned kref is meant to be passed to `kslot()` so a kernel + * service method can return the object to a vat, which receives it as + * an ordinary Presence. + * + * The object is pinned, so it stays alive until + * `releaseAnonymousKernelObject` is called; the registrar owns that + * lifetime. + * + * @param service - The object to host. + * @param label - A diagnostic label. Need not be unique; it is never + * used for lookup. + * @returns The kref of the newly hosted object. + */ + registerAnonymousKernelObject(service: object, label: string): KRef { + const kref = this.#kernelStore.initKernelObject('kernel'); + this.#kernelStore.pinObject(kref); + this.#kernelServicesByObject.set(kref, { + name: label, + kref, + service, + systemOnly: false, + }); + return kref; + } + + /** + * Release an object registered with `registerAnonymousKernelObject`, + * unpinning it and removing it from the routing table. Idempotent, and + * safe to call for a kref that was never registered. + * + * @param kref - The kref of the object to release. + */ + releaseAnonymousKernelObject(kref: KRef): void { + if (!this.#kernelServicesByObject.delete(kref)) { + return; + } + this.#kernelStore.unpinObject(kref); + } + /** * Get a kernel service by name. * From 2a4173adc102bec73e7d9792c5b709e5fe763832 Mon Sep 17 00:00:00 2001 From: Chip Morningstar Date: Tue, 4 Aug 2026 16:13:47 -0700 Subject: [PATCH 03/24] feat(ocap-kernel): IOListener with accept(), replacing single-client channels Splits the point of contact from the connection, BSD-style. An IOListener is what a cluster config's `io` entry now creates; its accept() yields one IOChannel per peer, each wrapped in its own exo and hosted as an anonymous kernel object, so the vat receives a Presence per connection. Sessions are isolated because they are distinct objects: holding one connection conveys no way to reach another, and `direction` is enforced per connection. IOManager tracks accepted connections per subcluster and releases them when the subcluster (or the listener) goes away. accept() resolves null once the listener is closed, so an accept loop can terminate rather than hang. **BREAKING:** Kernel's `ioChannelFactory` option becomes `ioListenerFactory`, and `IOChannelFactory` is replaced by `IOListener`/`IOListenerFactory`. Co-Authored-By: Claude Opus 4.7 --- packages/ocap-kernel/src/Kernel.ts | 22 +- packages/ocap-kernel/src/index.ts | 2 +- packages/ocap-kernel/src/io/IOManager.test.ts | 167 ++++++--- packages/ocap-kernel/src/io/IOManager.ts | 106 ++++-- packages/ocap-kernel/src/io/index.ts | 2 +- .../ocap-kernel/src/io/io-service.test.ts | 322 +++++++++++++++--- packages/ocap-kernel/src/io/io-service.ts | 112 +++++- packages/ocap-kernel/src/io/types.ts | 41 ++- 8 files changed, 625 insertions(+), 149 deletions(-) diff --git a/packages/ocap-kernel/src/Kernel.ts b/packages/ocap-kernel/src/Kernel.ts index 6cff649624..1139f4d4aa 100644 --- a/packages/ocap-kernel/src/Kernel.ts +++ b/packages/ocap-kernel/src/Kernel.ts @@ -4,7 +4,7 @@ import { isCapData } from '@metamask/kernel-utils'; import { Logger } from '@metamask/logger'; import { IOManager } from './io/IOManager.ts'; -import type { IOChannelFactory } from './io/types.ts'; +import type { IOListenerFactory } from './io/types.ts'; import { makeKernelFacet } from './kernel-facet.ts'; import type { KernelFacet } from './kernel-facet.ts'; import { KernelQueue } from './KernelQueue.ts'; @@ -103,7 +103,7 @@ export class Kernel { * @param options.logger - Optional logger for error and diagnostic output. * @param options.keySeed - Optional seed for libp2p key generation. * @param options.mnemonic - Optional BIP39 mnemonic for deriving the kernel identity. - * @param options.ioChannelFactory - Optional factory for creating IO channels. + * @param options.ioListenerFactory - Optional factory for creating IO listeners. * @param options.allowedGlobalNames - Optional list of allowed global names for vat endowments. */ // eslint-disable-next-line no-restricted-syntax @@ -115,7 +115,7 @@ export class Kernel { logger?: Logger; keySeed?: string | undefined; mnemonic?: string | undefined; - ioChannelFactory?: IOChannelFactory; + ioListenerFactory?: IOListenerFactory; allowedGlobalNames?: AllowedGlobalName[]; } = {}, ) { @@ -170,9 +170,9 @@ export class Kernel { logger: this.#logger.subLogger({ tags: ['KernelServiceManager'] }), }); - if (options.ioChannelFactory) { + if (options.ioListenerFactory) { this.#ioManager = new IOManager({ - factory: options.ioChannelFactory, + factory: options.ioListenerFactory, registerService: this.#kernelServiceManager.registerKernelServiceObject.bind( this.#kernelServiceManager, @@ -181,6 +181,14 @@ export class Kernel { this.#kernelServiceManager.unregisterKernelServiceObject.bind( this.#kernelServiceManager, ), + registerAnonymous: + this.#kernelServiceManager.registerAnonymousKernelObject.bind( + this.#kernelServiceManager, + ), + releaseAnonymous: + this.#kernelServiceManager.releaseAnonymousKernelObject.bind( + this.#kernelServiceManager, + ), logger: this.#logger.subLogger({ tags: ['IOManager'] }), }); } @@ -231,7 +239,7 @@ export class Kernel { * @param options.logger - Optional logger for error and diagnostic output. * @param options.keySeed - Optional seed for libp2p key generation. * @param options.mnemonic - Optional BIP39 mnemonic for deriving the kernel identity. - * @param options.ioChannelFactory - Optional factory for creating IO channels. + * @param options.ioListenerFactory - Optional factory for creating IO listeners. * @param options.systemSubclusters - Optional array of system subcluster configurations. * @param options.allowedGlobalNames - Optional list of allowed global names for vat endowments. When set, only these names from the `VatSupervisor`'s configured endowments (see `createDefaultEndowments`) are available to vats. * @returns A promise for the new kernel instance. @@ -244,7 +252,7 @@ export class Kernel { logger?: Logger; keySeed?: string | undefined; mnemonic?: string | undefined; - ioChannelFactory?: IOChannelFactory; + ioListenerFactory?: IOListenerFactory; systemSubclusters?: SystemSubclusterConfig[]; allowedGlobalNames?: AllowedGlobalName[]; } = {}, diff --git a/packages/ocap-kernel/src/index.ts b/packages/ocap-kernel/src/index.ts index 33bbbcab84..21fcd8d1be 100644 --- a/packages/ocap-kernel/src/index.ts +++ b/packages/ocap-kernel/src/index.ts @@ -8,7 +8,7 @@ export type { VatEndowments, } from './vats/endowments.ts'; export { initTransport } from './remotes/platform/transport.ts'; -export type { IOChannel, IOChannelFactory } from './io/types.ts'; +export type { IOChannel, IOListener, IOListenerFactory } from './io/types.ts'; export type { Baggage, ClusterConfig, diff --git a/packages/ocap-kernel/src/io/IOManager.test.ts b/packages/ocap-kernel/src/io/IOManager.test.ts index 31fe62712d..592452fd2b 100644 --- a/packages/ocap-kernel/src/io/IOManager.test.ts +++ b/packages/ocap-kernel/src/io/IOManager.test.ts @@ -2,9 +2,9 @@ import { Logger } from '@metamask/logger'; import { describe, it, expect, vi, beforeEach } from 'vitest'; import { IOManager } from './IOManager.ts'; -import type { IOChannel, IOChannelFactory } from './types.ts'; +import type { IOChannel, IOListener, IOListenerFactory } from './types.ts'; import type { KernelService } from '../KernelServiceManager.ts'; -import type { IOConfig } from '../types.ts'; +import type { IOConfig, KRef } from '../types.ts'; const makeChannel = (): IOChannel => ({ read: vi.fn().mockResolvedValue('data'), @@ -12,43 +12,61 @@ const makeChannel = (): IOChannel => ({ close: vi.fn().mockResolvedValue(undefined), }); +const makeListener = (): IOListener => ({ + accept: vi.fn().mockImplementation(async () => makeChannel()), + close: vi.fn().mockResolvedValue(undefined), +}); + +/** The vat-facing shape of a listener service, for driving it in tests. */ +type ListenerFacet = { accept: () => Promise }; + describe('IOManager', () => { - let factory: IOChannelFactory; + let factory: IOListenerFactory; let registerService: ReturnType; let unregisterService: ReturnType; + let registerAnonymous: ReturnType; + let releaseAnonymous: ReturnType; let logger: Logger; let manager: IOManager; - let channels: IOChannel[]; + let listeners: IOListener[]; + let registeredServices: Map; + let nextAnonymousId: number; beforeEach(() => { - channels = []; + listeners = []; + registeredServices = new Map(); + nextAnonymousId = 0; + factory = vi.fn(async () => { - const ch = makeChannel(); - channels.push(ch); - return ch; - }) as unknown as IOChannelFactory; - - registerService = vi.fn( - (name: string): KernelService => ({ - name, - kref: `ko${name}`, - service: {}, - systemOnly: false, - }), - ); + const listener = makeListener(); + listeners.push(listener); + return listener; + }) as unknown as IOListenerFactory; + + registerService = vi.fn((name: string, service: object): KernelService => { + registeredServices.set(name, service as unknown as ListenerFacet); + return { name, kref: `ko${name}`, service, systemOnly: false }; + }); unregisterService = vi.fn(); + registerAnonymous = vi.fn((): KRef => { + nextAnonymousId += 1; + return `ko${900 + nextAnonymousId}` as KRef; + }); + releaseAnonymous = vi.fn(); logger = new Logger('test'); manager = new IOManager({ factory, registerService, unregisterService, + registerAnonymous, + releaseAnonymous, logger, }); }); describe('createChannels', () => { - it('creates channels and registers services', async () => { + it('creates listeners and registers services', async () => { const ioConfig: Record = { repl: { type: 'socket', path: '/tmp/repl.sock' } as IOConfig, }; @@ -62,7 +80,7 @@ describe('IOManager', () => { ); }); - it('creates multiple channels', async () => { + it('creates multiple listeners', async () => { const ioConfig: Record = { input: { type: 'socket', path: '/tmp/in.sock' } as IOConfig, output: { type: 'socket', path: '/tmp/out.sock' } as IOConfig, @@ -74,21 +92,36 @@ describe('IOManager', () => { expect(registerService).toHaveBeenCalledTimes(2); }); + it('hosts connections accepted through the registered service', async () => { + await manager.createChannels('s1', { + repl: { type: 'socket', path: '/tmp/repl.sock' } as IOConfig, + }); + + await registeredServices.get('io:s1:repl')?.accept(); + + expect(registerAnonymous).toHaveBeenCalledWith( + expect.any(Object), + 'io:s1:repl:c1', + ); + }); + it('cleans up on factory failure', async () => { - const successChannel = makeChannel(); + const successListener = makeListener(); let callCount = 0; const failingFactory = vi.fn(async () => { callCount += 1; if (callCount === 2) { throw new Error('factory error'); } - return successChannel; - }) as unknown as IOChannelFactory; + return successListener; + }) as unknown as IOListenerFactory; const mgr = new IOManager({ factory: failingFactory, registerService, unregisterService, + registerAnonymous, + releaseAnonymous, logger, }); @@ -101,20 +134,52 @@ describe('IOManager', () => { 'factory error', ); - expect(successChannel.close).toHaveBeenCalledOnce(); + expect(successListener.close).toHaveBeenCalledOnce(); expect(unregisterService).toHaveBeenCalledWith('io:s1:first'); }); + it('releases already-accepted connections on factory failure', async () => { + let callCount = 0; + const failingFactory = vi.fn(async () => { + callCount += 1; + if (callCount === 2) { + // Accept a connection on the first listener before the second + // listener's creation blows up, so rollback has something to undo. + await registeredServices.get('io:s1:first')?.accept(); + throw new Error('factory error'); + } + return makeListener(); + }) as unknown as IOListenerFactory; + + const mgr = new IOManager({ + factory: failingFactory, + registerService, + unregisterService, + registerAnonymous, + releaseAnonymous, + logger, + }); + + await expect( + mgr.createChannels('s1', { + first: { type: 'socket', path: '/tmp/a.sock' } as IOConfig, + second: { type: 'socket', path: '/tmp/b.sock' } as IOConfig, + }), + ).rejects.toThrow('factory error'); + + expect(releaseAnonymous).toHaveBeenCalledWith('ko901'); + }); + it('does not mask factory error when unregister fails during rollback', async () => { - const successChannel = makeChannel(); + const successListener = makeListener(); let callCount = 0; const failingFactory = vi.fn(async () => { callCount += 1; if (callCount === 2) { throw new Error('factory error'); } - return successChannel; - }) as unknown as IOChannelFactory; + return successListener; + }) as unknown as IOListenerFactory; const failingUnregister = vi.fn(() => { throw new Error('unregister boom'); @@ -125,6 +190,8 @@ describe('IOManager', () => { factory: failingFactory, registerService, unregisterService: failingUnregister, + registerAnonymous, + releaseAnonymous, logger, }); @@ -142,12 +209,12 @@ describe('IOManager', () => { 'Error unregistering IO service "io:s1:first":', expect.any(Error), ); - expect(successChannel.close).toHaveBeenCalledOnce(); + expect(successListener.close).toHaveBeenCalledOnce(); }); }); describe('destroyChannels', () => { - it('closes channels and unregisters services', async () => { + it('closes listeners and unregisters services', async () => { const ioConfig: Record = { repl: { type: 'socket', path: '/tmp/repl.sock' } as IOConfig, }; @@ -155,10 +222,24 @@ describe('IOManager', () => { await manager.createChannels('s1', ioConfig); await manager.destroyChannels('s1'); - expect(channels[0]?.close).toHaveBeenCalledOnce(); + expect(listeners[0]?.close).toHaveBeenCalledOnce(); expect(unregisterService).toHaveBeenCalledWith('io:s1:repl'); }); + it('releases connections still accepted from the listener', async () => { + await manager.createChannels('s1', { + repl: { type: 'socket', path: '/tmp/repl.sock' } as IOConfig, + }); + const service = registeredServices.get('io:s1:repl'); + await service?.accept(); + await service?.accept(); + + await manager.destroyChannels('s1'); + + expect(releaseAnonymous).toHaveBeenCalledWith('ko901'); + expect(releaseAnonymous).toHaveBeenCalledWith('ko902'); + }); + it('is idempotent for unknown subcluster', async () => { expect(await manager.destroyChannels('nonexistent')).toBeUndefined(); }); @@ -173,6 +254,8 @@ describe('IOManager', () => { factory, registerService, unregisterService: failingUnregister, + registerAnonymous, + releaseAnonymous, logger, }); @@ -185,25 +268,27 @@ describe('IOManager', () => { 'Error unregistering IO service "io:s1:ch":', expect.any(Error), ); - // Channel should still be closed despite unregister failure - expect(channels[0]?.close).toHaveBeenCalledOnce(); + // Listener should still be closed despite unregister failure + expect(listeners[0]?.close).toHaveBeenCalledOnce(); }); it('handles close errors gracefully', async () => { - const errorChannel = makeChannel(); - (errorChannel.close as ReturnType).mockRejectedValue( + const errorListener = makeListener(); + (errorListener.close as ReturnType).mockRejectedValue( new Error('close failed'), ); const errorFactory = vi.fn( - async () => errorChannel, - ) as unknown as IOChannelFactory; + async () => errorListener, + ) as unknown as IOListenerFactory; const errorSpy = vi.spyOn(logger, 'error'); const mgr = new IOManager({ factory: errorFactory, registerService, unregisterService, + registerAnonymous, + releaseAnonymous, logger, }); @@ -213,14 +298,14 @@ describe('IOManager', () => { await mgr.destroyChannels('s1'); expect(errorSpy).toHaveBeenCalledWith( - 'Error closing IO channel "ch":', + 'Error closing IO listener "ch":', expect.any(Error), ); }); }); describe('destroyAllChannels', () => { - it('destroys channels for all subclusters', async () => { + it('destroys listeners for all subclusters', async () => { await manager.createChannels('s1', { a: { type: 'socket', path: '/tmp/a.sock' } as IOConfig, }); @@ -230,13 +315,13 @@ describe('IOManager', () => { await manager.destroyAllChannels(); - expect(channels[0]?.close).toHaveBeenCalledOnce(); - expect(channels[1]?.close).toHaveBeenCalledOnce(); + expect(listeners[0]?.close).toHaveBeenCalledOnce(); + expect(listeners[1]?.close).toHaveBeenCalledOnce(); expect(unregisterService).toHaveBeenCalledWith('io:s1:a'); expect(unregisterService).toHaveBeenCalledWith('io:s2:b'); }); - it('is safe to call when no channels exist', async () => { + it('is safe to call when no listeners exist', async () => { expect(await manager.destroyAllChannels()).toBeUndefined(); }); }); diff --git a/packages/ocap-kernel/src/io/IOManager.ts b/packages/ocap-kernel/src/io/IOManager.ts index cd3cdfdc6e..e445c670f7 100644 --- a/packages/ocap-kernel/src/io/IOManager.ts +++ b/packages/ocap-kernel/src/io/IOManager.ts @@ -1,9 +1,9 @@ import type { Logger } from '@metamask/logger'; -import { makeIOService } from './io-service.ts'; -import type { IOChannel, IOChannelFactory } from './types.ts'; +import { makeIOListenerService } from './io-service.ts'; +import type { IOListener, IOListenerFactory } from './types.ts'; import type { KernelService } from '../KernelServiceManager.ts'; -import type { IOConfig } from '../types.ts'; +import type { IOConfig, KRef } from '../types.ts'; type RegisterService = ( name: string, @@ -11,30 +11,41 @@ type RegisterService = ( options?: { systemOnly?: boolean }, ) => KernelService; type UnregisterService = (name: string) => void; +type RegisterAnonymous = (service: object, label: string) => KRef; +type ReleaseAnonymous = (kref: KRef) => void; type IOManagerOptions = { - factory: IOChannelFactory; + factory: IOListenerFactory; registerService: RegisterService; unregisterService: UnregisterService; + registerAnonymous: RegisterAnonymous; + releaseAnonymous: ReleaseAnonymous; logger?: Logger; }; type SubclusterIOState = { - channels: Map; + listeners: Map; serviceNames: string[]; + /** Krefs of connections accepted from this subcluster's listeners. */ + connectionKrefs: Set; }; /** - * Manages IO channel lifecycle, creating channels at subcluster launch - * and destroying them at termination. + * Manages IO listener lifecycle, creating listeners at subcluster launch + * and destroying them — along with any connections accepted from them — at + * termination. */ export class IOManager { - readonly #factory: IOChannelFactory; + readonly #factory: IOListenerFactory; readonly #registerService: RegisterService; readonly #unregisterService: UnregisterService; + readonly #registerAnonymous: RegisterAnonymous; + + readonly #releaseAnonymous: ReleaseAnonymous; + readonly #logger: Logger | undefined; /** IO state indexed by subcluster ID */ @@ -44,53 +55,75 @@ export class IOManager { * Creates a new IOManager instance. * * @param options - Constructor options. - * @param options.factory - Factory for creating IO channels. + * @param options.factory - Factory for creating IO listeners. * @param options.registerService - Function to register a kernel service. * @param options.unregisterService - Function to unregister a kernel service. + * @param options.registerAnonymous - Function to host an accepted + * connection as a kernel object reachable only by reference. + * @param options.releaseAnonymous - Function to release a hosted connection. * @param options.logger - Optional logger for diagnostics. */ constructor({ factory, registerService, unregisterService, + registerAnonymous, + releaseAnonymous, logger, }: IOManagerOptions) { this.#factory = factory; this.#registerService = registerService; this.#unregisterService = unregisterService; + this.#registerAnonymous = registerAnonymous; + this.#releaseAnonymous = releaseAnonymous; this.#logger = logger; harden(this); } /** - * Create IO channels for a subcluster and register them as kernel services. + * Create IO listeners for a subcluster and register them as kernel services. * * @param subclusterId - The ID of the subcluster. - * @param ioConfig - The IO configuration map from channel names to configs. + * @param ioConfig - The IO configuration map from listener names to configs. */ async createChannels( subclusterId: string, ioConfig: Record, ): Promise { - const channels = new Map(); + const listeners = new Map(); const serviceNames: string[] = []; + const connectionKrefs = new Set(); for (const [name, config] of Object.entries(ioConfig)) { const serviceName = `io:${subclusterId}:${name}`; try { - const channel = await this.#factory(name, config); - channels.set(name, channel); - - const service = makeIOService(serviceName, channel, config); + const listener = await this.#factory(name, config); + listeners.set(name, listener); + + const service = makeIOListenerService(serviceName, listener, config, { + register: (connection, label) => { + const kref = this.#registerAnonymous(connection, label); + connectionKrefs.add(kref); + return kref; + }, + release: (kref) => { + connectionKrefs.delete(kref); + this.#releaseAnonymous(kref); + }, + }); this.#registerService(serviceName, service); serviceNames.push(serviceName); this.#logger?.info( - `Created IO channel "${name}" for subcluster ${subclusterId}`, + `Created IO listener "${name}" for subcluster ${subclusterId}`, ); } catch (error) { - // Clean up any channels we already created before re-throwing - await this.#closeChannels(channels); + // Clean up anything we already created before re-throwing + await this.#closeListeners(listeners); + for (const kref of connectionKrefs) { + this.#releaseAnonymous(kref); + } + connectionKrefs.clear(); for (const registeredName of serviceNames) { try { this.#unregisterService(registeredName); @@ -105,11 +138,16 @@ export class IOManager { } } - this.#subclusters.set(subclusterId, { channels, serviceNames }); + this.#subclusters.set(subclusterId, { + listeners, + serviceNames, + connectionKrefs, + }); } /** - * Destroy IO channels for a subcluster and unregister their services. + * Destroy IO listeners for a subcluster, unregister their services, and + * release any connections still accepted from them. * * @param subclusterId - The ID of the subcluster. */ @@ -127,15 +165,21 @@ export class IOManager { } } - await this.#closeChannels(state.channels); + // Closing a listener closes its connections at the transport level; + // stop hosting them so their krefs don't outlive the subcluster. + await this.#closeListeners(state.listeners); + for (const kref of state.connectionKrefs) { + this.#releaseAnonymous(kref); + } + state.connectionKrefs.clear(); this.#subclusters.delete(subclusterId); - this.#logger?.info(`Destroyed IO channels for subcluster ${subclusterId}`); + this.#logger?.info(`Destroyed IO listeners for subcluster ${subclusterId}`); } /** - * Destroy all IO channels across all subclusters. - * Used during kernel reset to ensure no channels are leaked. + * Destroy all IO listeners across all subclusters. + * Used during kernel reset to ensure nothing is leaked. */ async destroyAllChannels(): Promise { for (const subclusterId of [...this.#subclusters.keys()]) { @@ -144,16 +188,16 @@ export class IOManager { } /** - * Close all channels in a map, logging errors. + * Close all listeners in a map, logging errors. * - * @param channels - The channels to close. + * @param listeners - The listeners to close. */ - async #closeChannels(channels: Map): Promise { - for (const [name, channel] of channels) { + async #closeListeners(listeners: Map): Promise { + for (const [name, listener] of listeners) { try { - await channel.close(); + await listener.close(); } catch (error) { - this.#logger?.error(`Error closing IO channel "${name}":`, error); + this.#logger?.error(`Error closing IO listener "${name}":`, error); } } } diff --git a/packages/ocap-kernel/src/io/index.ts b/packages/ocap-kernel/src/io/index.ts index a132c8a31a..033e1fe200 100644 --- a/packages/ocap-kernel/src/io/index.ts +++ b/packages/ocap-kernel/src/io/index.ts @@ -1,2 +1,2 @@ export { IOManager } from './IOManager.ts'; -export type { IOChannel, IOChannelFactory } from './types.ts'; +export type { IOChannel, IOListener, IOListenerFactory } from './types.ts'; diff --git a/packages/ocap-kernel/src/io/io-service.test.ts b/packages/ocap-kernel/src/io/io-service.test.ts index 14c8c0a574..4b760e2283 100644 --- a/packages/ocap-kernel/src/io/io-service.test.ts +++ b/packages/ocap-kernel/src/io/io-service.test.ts @@ -1,8 +1,25 @@ import { describe, it, expect, vi } from 'vitest'; -import { makeIOService } from './io-service.ts'; -import type { IOChannel } from './types.ts'; -import type { IOConfig } from '../types.ts'; +import { + makeIOConnectionService, + makeIOListenerService, +} from './io-service.ts'; +import type { ConnectionHost } from './io-service.ts'; +import type { IOChannel, IOListener } from './types.ts'; +import { krefOf } from '../liveslots/kernel-marshal.ts'; +import type { SlotValue } from '../liveslots/kernel-marshal.ts'; +import type { IOConfig, KRef } from '../types.ts'; + +type ConnectionFacet = { + read: () => Promise; + write: (data: string) => Promise; + close: () => Promise; +}; + +type ListenerFacet = { + accept: () => Promise; + close: () => Promise; +}; const makeChannel = (): IOChannel => ({ read: vi.fn().mockResolvedValue('hello'), @@ -17,34 +34,75 @@ const makeConfig = (overrides: Partial = {}): IOConfig => ...overrides, }) as IOConfig; -describe('makeIOService', () => { +/** + * Build a listener that hands out the supplied channels in order, then + * reports EOF by resolving `null`. + * + * @param channels - The channels to yield from successive `accept()` calls. + * @returns The listener plus its close spy. + */ +function makeListener(channels: IOChannel[]): IOListener { + const queue = [...channels]; + return { + accept: vi.fn().mockImplementation(async () => queue.shift() ?? null), + close: vi.fn().mockResolvedValue(undefined), + }; +} + +/** + * Build a connection host that allocates sequential fake krefs and records + * what was registered and released. + * + * @returns The host plus its bookkeeping. + */ +function makeHost(): ConnectionHost & { + registered: { kref: KRef; label: string; connection: object }[]; + released: KRef[]; +} { + const registered: { kref: KRef; label: string; connection: object }[] = []; + const released: KRef[] = []; + let next = 0; + return { + registered, + released, + register: (connection: object, label: string): KRef => { + next += 1; + const kref = `ko${next}` as KRef; + registered.push({ kref, label, connection }); + return kref; + }, + release: (kref: KRef): void => { + released.push(kref); + }, + }; +} + +describe('makeIOConnectionService', () => { describe('read()', () => { it('delegates to the channel', async () => { const channel = makeChannel(); - const service = makeIOService( - 'io:subclusterFoo:test', + const connection = makeIOConnectionService( + 'io:subclusterFoo:test:c1', channel, makeConfig(), - ) as { - read: () => Promise; - }; + vi.fn(), + ) as ConnectionFacet; - const result = await service.read(); - - expect(result).toBe('hello'); + expect(await connection.read()).toBe('hello'); expect(channel.read).toHaveBeenCalledOnce(); }); - it('throws on write-only channel', async () => { + it('throws on a write-only connection', async () => { const channel = makeChannel(); - const service = makeIOService( - 'io:subclusterFoo:test', + const connection = makeIOConnectionService( + 'io:subclusterFoo:test:c1', channel, makeConfig({ direction: 'out' }), - ) as { read: () => Promise }; + vi.fn(), + ) as ConnectionFacet; - await expect(service.read()).rejects.toThrow( - 'IO channel "io:subclusterFoo:test" is write-only', + await expect(connection.read()).rejects.toThrow( + 'IO connection "io:subclusterFoo:test:c1" is write-only', ); expect(channel.read).not.toHaveBeenCalled(); }); @@ -52,14 +110,14 @@ describe('makeIOService', () => { it.each(['in', 'inout'] as const)( 'allows read on direction=%s', async (direction) => { - const channel = makeChannel(); - const service = makeIOService( - 'io:subclusterFoo:test', - channel, + const connection = makeIOConnectionService( + 'io:subclusterFoo:test:c1', + makeChannel(), makeConfig({ direction }), - ) as { read: () => Promise }; + vi.fn(), + ) as ConnectionFacet; - expect(await service.read()).toBe('hello'); + expect(await connection.read()).toBe('hello'); }, ); }); @@ -67,29 +125,29 @@ describe('makeIOService', () => { describe('write()', () => { it('delegates to the channel', async () => { const channel = makeChannel(); - const service = makeIOService( - 'io:subclusterFoo:test', + const connection = makeIOConnectionService( + 'io:subclusterFoo:test:c1', channel, makeConfig(), - ) as { - write: (data: string) => Promise; - }; + vi.fn(), + ) as ConnectionFacet; - await service.write('world'); + await connection.write('world'); expect(channel.write).toHaveBeenCalledWith('world'); }); - it('throws on read-only channel', async () => { + it('throws on a read-only connection', async () => { const channel = makeChannel(); - const service = makeIOService( - 'io:subclusterFoo:test', + const connection = makeIOConnectionService( + 'io:subclusterFoo:test:c1', channel, makeConfig({ direction: 'in' }), - ) as { write: (data: string) => Promise }; + vi.fn(), + ) as ConnectionFacet; - await expect(service.write('data')).rejects.toThrow( - 'IO channel "io:subclusterFoo:test" is read-only', + await expect(connection.write('data')).rejects.toThrow( + 'IO connection "io:subclusterFoo:test:c1" is read-only', ); expect(channel.write).not.toHaveBeenCalled(); }); @@ -97,32 +155,192 @@ describe('makeIOService', () => { it.each(['out', 'inout'] as const)( 'allows write on direction=%s', async (direction) => { - const channel = makeChannel(); - const service = makeIOService( - 'io:subclusterFoo:test', - channel, + const connection = makeIOConnectionService( + 'io:subclusterFoo:test:c1', + makeChannel(), makeConfig({ direction }), - ) as { write: (data: string) => Promise }; + vi.fn(), + ) as ConnectionFacet; - expect(await service.write('data')).toBeUndefined(); + expect(await connection.write('data')).toBeUndefined(); }, ); }); - describe('direction defaults', () => { - it('defaults to inout when direction is not specified', async () => { + describe('close()', () => { + it('closes the channel and notifies the host', async () => { + const channel = makeChannel(); + const onClose = vi.fn(); + const connection = makeIOConnectionService( + 'io:subclusterFoo:test:c1', + channel, + makeConfig(), + onClose, + ) as ConnectionFacet; + + await connection.close(); + + expect(channel.close).toHaveBeenCalledOnce(); + expect(onClose).toHaveBeenCalledOnce(); + }); + + it('is idempotent', async () => { const channel = makeChannel(); - const service = makeIOService( - 'io:subclusterFoo:test', + const onClose = vi.fn(); + const connection = makeIOConnectionService( + 'io:subclusterFoo:test:c1', channel, makeConfig(), - ) as { - read: () => Promise; - write: (data: string) => Promise; - }; + onClose, + ) as ConnectionFacet; + + await connection.close(); + await connection.close(); + + expect(channel.close).toHaveBeenCalledOnce(); + expect(onClose).toHaveBeenCalledOnce(); + }); + + it('notifies the host even when the channel close fails', async () => { + const channel = makeChannel(); + (channel.close as unknown as ReturnType).mockRejectedValue( + new Error('boom'), + ); + const onClose = vi.fn(); + const connection = makeIOConnectionService( + 'io:subclusterFoo:test:c1', + channel, + makeConfig(), + onClose, + ) as ConnectionFacet; + + await expect(connection.close()).rejects.toThrow('boom'); + expect(onClose).toHaveBeenCalledOnce(); + }); + }); + + describe('direction defaults', () => { + it('defaults to inout when direction is not specified', async () => { + const connection = makeIOConnectionService( + 'io:subclusterFoo:test:c1', + makeChannel(), + makeConfig(), + vi.fn(), + ) as ConnectionFacet; - expect(await service.read()).toBe('hello'); - expect(await service.write('data')).toBeUndefined(); + expect(await connection.read()).toBe('hello'); + expect(await connection.write('data')).toBeUndefined(); }); }); }); + +describe('makeIOListenerService', () => { + it('hosts each accepted connection and returns a reference to it', async () => { + const channel = makeChannel(); + const host = makeHost(); + const listener = makeIOListenerService( + 'io:s1:repl', + makeListener([channel]), + makeConfig(), + host, + ) as ListenerFacet; + + const result = await listener.accept(); + + expect(host.registered).toHaveLength(1); + expect(host.registered[0]?.label).toBe('io:s1:repl:c1'); + // The vat receives a reference, never a raw name it could forge. + expect(krefOf(result as SlotValue)).toBe('ko1'); + }); + + it('gives each connection a distinct identity', async () => { + const host = makeHost(); + const listener = makeIOListenerService( + 'io:s1:repl', + makeListener([makeChannel(), makeChannel()]), + makeConfig(), + host, + ) as ListenerFacet; + + const first = await listener.accept(); + const second = await listener.accept(); + + expect(krefOf(first as SlotValue)).toBe('ko1'); + expect(krefOf(second as SlotValue)).toBe('ko2'); + expect(host.registered.map((entry) => entry.label)).toStrictEqual([ + 'io:s1:repl:c1', + 'io:s1:repl:c2', + ]); + }); + + it('isolates connections: each reads only its own channel', async () => { + const first = makeChannel(); + const second = makeChannel(); + (first.read as unknown as ReturnType).mockResolvedValue( + 'from-first', + ); + (second.read as unknown as ReturnType).mockResolvedValue( + 'from-second', + ); + const host = makeHost(); + const listener = makeIOListenerService( + 'io:s1:repl', + makeListener([first, second]), + makeConfig(), + host, + ) as ListenerFacet; + + await listener.accept(); + await listener.accept(); + + const facets = host.registered.map( + (entry) => entry.connection as unknown as ConnectionFacet, + ); + expect(await facets[0]?.read()).toBe('from-first'); + expect(await facets[1]?.read()).toBe('from-second'); + }); + + it('releases a connection from the host when it is closed', async () => { + const host = makeHost(); + const listener = makeIOListenerService( + 'io:s1:repl', + makeListener([makeChannel()]), + makeConfig(), + host, + ) as ListenerFacet; + + await listener.accept(); + const connection = host.registered[0] + ?.connection as unknown as ConnectionFacet; + await connection.close(); + + expect(host.released).toStrictEqual(['ko1']); + }); + + it('returns null once the listener is exhausted', async () => { + const host = makeHost(); + const listener = makeIOListenerService( + 'io:s1:repl', + makeListener([]), + makeConfig(), + host, + ) as ListenerFacet; + + expect(await listener.accept()).toBeNull(); + expect(host.registered).toHaveLength(0); + }); + + it('delegates close() to the listener', async () => { + const underlying = makeListener([]); + const listener = makeIOListenerService( + 'io:s1:repl', + underlying, + makeConfig(), + makeHost(), + ) as ListenerFacet; + + await listener.close(); + + expect(underlying.close).toHaveBeenCalledOnce(); + }); +}); diff --git a/packages/ocap-kernel/src/io/io-service.ts b/packages/ocap-kernel/src/io/io-service.ts index 0c7d86468e..b61b61d0c0 100644 --- a/packages/ocap-kernel/src/io/io-service.ts +++ b/packages/ocap-kernel/src/io/io-service.ts @@ -1,37 +1,129 @@ import { makeDefaultExo } from '@metamask/kernel-utils/exo'; -import type { IOChannel } from './types.ts'; +import type { IOChannel, IOListener } from './types.ts'; +import { kslot } from '../liveslots/kernel-marshal.ts'; +import type { KRef } from '../types.ts'; import type { IOConfig } from '../types.ts'; /** - * Create a kernel service exo that wraps an IOChannel. + * Hooks the listener service uses to host each accepted connection as a + * kernel object reachable only by reference. + */ +export type ConnectionHost = { + /** Host `connection` and return its kref. */ + register: (connection: object, label: string) => KRef; + /** Release a previously hosted connection. */ + release: (kref: KRef) => void; +}; + +/** + * Create a kernel service exo wrapping an `IOChannel` for one accepted + * connection. * - * @param name - The scoped service name (e.g. `io:s1:repl`). - * @param channel - The underlying IOChannel to delegate to. - * @param config - The IO configuration for this channel. - * @returns A remotable service object with `read()` and `write()` methods. + * `direction` is enforced here rather than on the listener, since it is a + * property of the data flow rather than of the point of contact. + * + * @param name - The scoped connection name, used as the exo's interface + * name (e.g. `io:s1:repl:c3`). + * @param channel - The channel for this connection. + * @param config - The IO configuration for the owning listener. + * @param onClose - Invoked after the channel closes, so the host can stop + * hosting this connection. + * @returns A remotable with `read()`, `write()`, and `close()`. */ -export function makeIOService( +export function makeIOConnectionService( name: string, channel: IOChannel, config: IOConfig, + onClose: () => void, ): object { const direction = config.direction ?? 'inout'; + let closed = false; return makeDefaultExo(name, { async read(): Promise { if (direction === 'out') { - throw new Error(`IO channel "${name}" is write-only`); + throw new Error(`IO connection "${name}" is write-only`); } return channel.read(); }, async write(data: string): Promise { if (direction === 'in') { - throw new Error(`IO channel "${name}" is read-only`); + throw new Error(`IO connection "${name}" is read-only`); } return channel.write(data); }, + + async close(): Promise { + if (closed) { + return; + } + closed = true; + try { + await channel.close(); + } finally { + onClose(); + } + }, + }); +} +harden(makeIOConnectionService); + +/** + * Create a kernel service exo that wraps an `IOListener`. + * + * `accept()` waits for the next peer, wraps that connection in its own exo, + * hosts it as an anonymous kernel object, and returns a `kslot` standin so + * the calling vat receives it as an ordinary Presence. Each connection is + * therefore a distinct object with its own state, and holding one conveys + * no access to any other. + * + * @param name - The scoped service name (e.g. `io:s1:repl`). + * @param listener - The underlying listener to delegate to. + * @param config - The IO configuration for this listener. + * @param host - Hooks for hosting accepted connections as kernel objects. + * @returns A remotable service object with `accept()` and `close()`. + */ +export function makeIOListenerService( + name: string, + listener: IOListener, + config: IOConfig, + host: ConnectionHost, +): object { + let nextConnectionId = 0; + + return makeDefaultExo(name, { + async accept(): Promise { + const channel = await listener.accept(); + if (!channel) { + // Listener closed; report EOF rather than leaving the caller's + // accept loop hanging forever. + return null; + } + nextConnectionId += 1; + const connectionName = `${name}:c${nextConnectionId}`; + // Hosting needs the connection object, but the connection's close + // handler needs the resulting kref, so the kref is shared through a + // holder that is filled in immediately after registration. + const hosted: { kref?: KRef } = {}; + const connection = makeIOConnectionService( + connectionName, + channel, + config, + () => { + if (hosted.kref) { + host.release(hosted.kref); + } + }, + ); + hosted.kref = host.register(connection, connectionName); + return kslot(hosted.kref, connectionName); + }, + + async close(): Promise { + return listener.close(); + }, }); } -harden(makeIOService); +harden(makeIOListenerService); diff --git a/packages/ocap-kernel/src/io/types.ts b/packages/ocap-kernel/src/io/types.ts index f08f00dbd6..44e316b90d 100644 --- a/packages/ocap-kernel/src/io/types.ts +++ b/packages/ocap-kernel/src/io/types.ts @@ -3,6 +3,10 @@ import type { IOConfig } from '../types.ts'; /** * A platform-agnostic IO channel that supports reading and writing data. * Implementations are platform-specific (e.g., Unix domain sockets in Node.js). + * + * A channel represents a *single* connection: one bidirectional stream of + * data with one peer. Serving several peers concurrently means holding + * several channels, one per peer, obtained from an `IOListener`. */ export type IOChannel = { /** Read the next unit of data, or `null` on EOF/disconnect. */ @@ -14,14 +18,39 @@ export type IOChannel = { }; /** - * Factory function that creates an IOChannel for a given configuration. + * A platform-agnostic endpoint that peers connect to, yielding one + * `IOChannel` per connection. + * + * This is the BSD listen/accept split: the listener is the stable, + * configured point of contact (one socket path, one entry in a cluster + * config's `io` map), while each accepted connection is a separate object + * with its own state. Sessions are isolated because they are distinct + * objects, so a holder of one connection has no way to reach another. + */ +export type IOListener = { + /** + * Wait for the next peer to connect and return a channel for it. + * + * Resolves to `null` once the listener has been closed, so an accept + * loop can terminate rather than hang. + */ + accept(): Promise; + /** + * Stop listening and close every connection accepted from this + * listener. + */ + close(): Promise; +}; + +/** + * Factory function that creates an IOListener for a given configuration. * Injected from the host environment (e.g., Node.js) into the kernel. * - * @param name - The name of the IO channel (from the cluster config key). - * @param config - The IO configuration describing the channel type and options. - * @returns A promise for the created IOChannel. + * @param name - The name of the IO listener (from the cluster config key). + * @param config - The IO configuration describing the listener type and options. + * @returns A promise for the created IOListener. */ -export type IOChannelFactory = ( +export type IOListenerFactory = ( name: string, config: IOConfig, -) => Promise; +) => Promise; From 43edc1a31de3a4ff46bec42330e559b4113ac63b Mon Sep 17 00:00:00 2001 From: Chip Morningstar Date: Tue, 4 Aug 2026 16:13:55 -0700 Subject: [PATCH 04/24] feat(kernel-node-runtime): socket listener with per-connection channels MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces makeSocketIOChannel with makeSocketIOListener. The server hands each connection to accept() as its own IOChannel whose buffer, decoder, line queue, and reader queue are all local to that connection, so any number of peers can be served at once. Connections that arrive before accept() is called are queued rather than dropped. Gone with the single-client design: currentSocket, pendingSessionEnd, the merged lineQueue, and the socket.destroy() that rejected every second connection. Session boundaries need no latch now — one channel serves one peer, so the end of the socket simply is the end of the channel. **BREAKING:** makeIOChannelFactory becomes makeIOListenerFactory; makeSocketIOChannel becomes makeSocketIOListener. Co-Authored-By: Claude Opus 4.7 --- packages/kernel-node-runtime/src/index.ts | 2 +- packages/kernel-node-runtime/src/io/index.ts | 18 +- .../src/io/socket-channel.test.ts | 301 ------------- .../src/io/socket-channel.ts | 191 --------- .../src/io/socket-listener.test.ts | 401 ++++++++++++++++++ .../src/io/socket-listener.ts | 253 +++++++++++ .../src/kernel/make-kernel.ts | 12 +- .../test/helpers/kernel.ts | 10 +- 8 files changed, 675 insertions(+), 513 deletions(-) delete mode 100644 packages/kernel-node-runtime/src/io/socket-channel.test.ts delete mode 100644 packages/kernel-node-runtime/src/io/socket-channel.ts create mode 100644 packages/kernel-node-runtime/src/io/socket-listener.test.ts create mode 100644 packages/kernel-node-runtime/src/io/socket-listener.ts diff --git a/packages/kernel-node-runtime/src/index.ts b/packages/kernel-node-runtime/src/index.ts index 1a1eeb323c..a05ad038f8 100644 --- a/packages/kernel-node-runtime/src/index.ts +++ b/packages/kernel-node-runtime/src/index.ts @@ -2,4 +2,4 @@ export { NodejsPlatformServices } from './kernel/PlatformServices.ts'; export { makeKernel } from './kernel/make-kernel.ts'; export type { MakeKernelResult } from './kernel/make-kernel.ts'; export { makeNodeJsVatSupervisor } from './vat/make-supervisor.ts'; -export { makeIOChannelFactory, makeSocketIOChannel } from './io/index.ts'; +export { makeIOListenerFactory, makeSocketIOListener } from './io/index.ts'; diff --git a/packages/kernel-node-runtime/src/io/index.ts b/packages/kernel-node-runtime/src/io/index.ts index 739aa7757d..2bdfd38e2e 100644 --- a/packages/kernel-node-runtime/src/io/index.ts +++ b/packages/kernel-node-runtime/src/io/index.ts @@ -1,23 +1,23 @@ -import type { IOChannelFactory, IOConfig } from '@metamask/ocap-kernel'; +import type { IOListenerFactory, IOConfig } from '@metamask/ocap-kernel'; -import { makeSocketIOChannel } from './socket-channel.ts'; +import { makeSocketIOListener } from './socket-listener.ts'; -export { makeSocketIOChannel } from './socket-channel.ts'; +export { makeSocketIOListener } from './socket-listener.ts'; /** - * Create an IOChannelFactory for the Node.js environment. - * Dispatches on `config.type` to the appropriate channel implementation. + * Create an IOListenerFactory for the Node.js environment. + * Dispatches on `config.type` to the appropriate listener implementation. * - * @returns An IOChannelFactory. + * @returns An IOListenerFactory. */ -export function makeIOChannelFactory(): IOChannelFactory { +export function makeIOListenerFactory(): IOListenerFactory { return async (name: string, config: IOConfig) => { switch (config.type) { case 'socket': - return makeSocketIOChannel(name, config.path); + return makeSocketIOListener(name, config.path); default: throw new Error( - `Unsupported IO channel type "${config.type}" for channel "${name}"`, + `Unsupported IO listener type "${config.type}" for listener "${name}"`, ); } }; diff --git a/packages/kernel-node-runtime/src/io/socket-channel.test.ts b/packages/kernel-node-runtime/src/io/socket-channel.test.ts deleted file mode 100644 index fe8bf982c9..0000000000 --- a/packages/kernel-node-runtime/src/io/socket-channel.test.ts +++ /dev/null @@ -1,301 +0,0 @@ -import type { IOChannel } from '@metamask/ocap-kernel'; -import fs from 'node:fs/promises'; -import * as net from 'node:net'; -import * as os from 'node:os'; -import * as path from 'node:path'; -import { describe, it, expect, afterEach } from 'vitest'; - -import { makeSocketIOChannel } from './socket-channel.ts'; - -function tempSocketPath(): string { - return path.join( - os.tmpdir(), - `io-test-${Date.now()}-${Math.random().toString(36).slice(2)}.sock`, - ); -} - -async function connectToSocket(socketPath: string): Promise { - return new Promise((resolve, reject) => { - const client = net.createConnection(socketPath, () => { - client.removeListener('error', reject); - resolve(client); - }); - client.on('error', reject); - }); -} - -async function writeLine(socket: net.Socket, line: string): Promise { - return new Promise((resolve, reject) => { - socket.write(`${line}\n`, (error) => { - if (error) { - reject(error); - } else { - resolve(); - } - }); - }); -} - -async function readLine(socket: net.Socket): Promise { - return new Promise((resolve) => { - let buffer = ''; - const onData = (data: Buffer): void => { - buffer += data.toString(); - const idx = buffer.indexOf('\n'); - if (idx !== -1) { - socket.removeListener('data', onData); - resolve(buffer.slice(0, idx)); - } - }; - socket.on('data', onData); - }); -} - -async function fileExists(filePath: string): Promise { - try { - await fs.access(filePath); - return true; - } catch { - return false; - } -} - -describe('makeSocketIOChannel', () => { - const channels: IOChannel[] = []; - const clients: net.Socket[] = []; - - afterEach(async () => { - for (const client of clients) { - client.destroy(); - } - clients.length = 0; - for (const channel of channels) { - await channel.close(); - } - channels.length = 0; - }); - - it('creates a listening socket', async () => { - const socketPath = tempSocketPath(); - const channel = await makeSocketIOChannel('test', socketPath); - channels.push(channel); - - expect(await fileExists(socketPath)).toBe(true); - }); - - it('reads lines from a connected client', async () => { - const socketPath = tempSocketPath(); - const channel = await makeSocketIOChannel('test', socketPath); - channels.push(channel); - - const client = await connectToSocket(socketPath); - clients.push(client); - - await writeLine(client, 'hello'); - await writeLine(client, 'world'); - - const line1 = await channel.read(); - const line2 = await channel.read(); - - expect(line1).toBe('hello'); - expect(line2).toBe('world'); - }); - - it('writes lines to a connected client', async () => { - const socketPath = tempSocketPath(); - const channel = await makeSocketIOChannel('test', socketPath); - channels.push(channel); - - const client = await connectToSocket(socketPath); - clients.push(client); - - // Small delay for connection to be established - await new Promise((resolve) => setTimeout(resolve, 10)); - - const linePromise = readLine(client); - await channel.write('output'); - const received = await linePromise; - - expect(received).toBe('output'); - }); - - it('returns null on client disconnect', async () => { - const socketPath = tempSocketPath(); - const channel = await makeSocketIOChannel('test', socketPath); - channels.push(channel); - - const client = await connectToSocket(socketPath); - - // Start a read that will block - const readPromise = channel.read(); - client.destroy(); - - const result = await readPromise; - expect(result).toBeNull(); - }); - - it('blocks read until a client connects and sends data', async () => { - const socketPath = tempSocketPath(); - const channel = await makeSocketIOChannel('test', socketPath); - channels.push(channel); - - // Start read before any client connects — should block - const readPromise = channel.read(); - - // Connect and send data - const client = await connectToSocket(socketPath); - clients.push(client); - await writeLine(client, 'hello'); - - const result = await readPromise; - expect(result).toBe('hello'); - }); - - it('throws on write when no client is connected', async () => { - const socketPath = tempSocketPath(); - const channel = await makeSocketIOChannel('test', socketPath); - channels.push(channel); - - await expect(channel.write('data')).rejects.toThrow( - 'has no connected client', - ); - }); - - it('queues lines before read is called', async () => { - const socketPath = tempSocketPath(); - const channel = await makeSocketIOChannel('test', socketPath); - channels.push(channel); - - const client = await connectToSocket(socketPath); - clients.push(client); - - // Send lines before any reads - await writeLine(client, 'a'); - await writeLine(client, 'b'); - - // Small delay for data to arrive - await new Promise((resolve) => setTimeout(resolve, 20)); - - expect(await channel.read()).toBe('a'); - expect(await channel.read()).toBe('b'); - }); - - it('rejects second connection', async () => { - const socketPath = tempSocketPath(); - const channel = await makeSocketIOChannel('test', socketPath); - channels.push(channel); - - const client1 = await connectToSocket(socketPath); - clients.push(client1); - - const client2 = await connectToSocket(socketPath); - - // Second client should be destroyed - await new Promise((resolve) => { - client2.on('close', () => resolve()); - }); - expect(client2.destroyed).toBe(true); - }); - - it('cleans up socket file on close', async () => { - const socketPath = tempSocketPath(); - const channel = await makeSocketIOChannel('test', socketPath); - - expect(await fileExists(socketPath)).toBe(true); - await channel.close(); - expect(await fileExists(socketPath)).toBe(false); - }); - - it('returns null after close', async () => { - const socketPath = tempSocketPath(); - const channel = await makeSocketIOChannel('test', socketPath); - - await channel.close(); - - const result = await channel.read(); - expect(result).toBeNull(); - }); - - it('throws on write after close', async () => { - const socketPath = tempSocketPath(); - const channel = await makeSocketIOChannel('test', socketPath); - const client = await connectToSocket(socketPath); - clients.push(client); - - await channel.close(); - - await expect(channel.write('data')).rejects.toThrow('is closed'); - }); - - it('drains stale lineQueue when a new client connects', async () => { - const socketPath = tempSocketPath(); - const channel = await makeSocketIOChannel('test', socketPath); - channels.push(channel); - - // First client sends lines that are not read - const client1 = await connectToSocket(socketPath); - await writeLine(client1, 'stale-line'); - await new Promise((resolve) => setTimeout(resolve, 20)); - - // Disconnect first client - client1.destroy(); - await new Promise((resolve) => setTimeout(resolve, 20)); - - // Second client connects — stale lines should be gone - const client2 = await connectToSocket(socketPath); - clients.push(client2); - - await writeLine(client2, 'fresh-line'); - await new Promise((resolve) => setTimeout(resolve, 20)); - - expect(await channel.read()).toBe('fresh-line'); - }); - - it('handles multi-byte UTF-8 split across TCP chunks', async () => { - const socketPath = tempSocketPath(); - const channel = await makeSocketIOChannel('test', socketPath); - channels.push(channel); - - const client = await connectToSocket(socketPath); - clients.push(client); - - // U+1F600 (😀) is 4 bytes: f0 9f 98 80 - const emoji = '\u{1F600}'; - const fullMessage = `hello ${emoji} world\n`; - const encoded = Buffer.from(fullMessage, 'utf8'); - - // Split in the middle of the emoji (after first 2 bytes of the 4-byte sequence) - const splitPoint = Buffer.from('hello ', 'utf8').length + 2; - const chunk1 = encoded.subarray(0, splitPoint); - const chunk2 = encoded.subarray(splitPoint); - - // Send the two chunks separately - await new Promise((resolve, reject) => { - client.write(chunk1, (error) => (error ? reject(error) : resolve())); - }); - await new Promise((resolve) => setTimeout(resolve, 10)); - await new Promise((resolve, reject) => { - client.write(chunk2, (error) => (error ? reject(error) : resolve())); - }); - await new Promise((resolve) => setTimeout(resolve, 10)); - - expect(await channel.read()).toBe(`hello ${emoji} world`); - }); - - it('removes stale socket file on creation', async () => { - const socketPath = tempSocketPath(); - - // Create the first channel - const channel1 = await makeSocketIOChannel('test', socketPath); - await channel1.close(); - - // Recreate a stale file - await fs.writeFile(socketPath, ''); - - // Should succeed despite the stale file - const channel2 = await makeSocketIOChannel('test', socketPath); - channels.push(channel2); - - expect(await fileExists(socketPath)).toBe(true); - }); -}); diff --git a/packages/kernel-node-runtime/src/io/socket-channel.ts b/packages/kernel-node-runtime/src/io/socket-channel.ts deleted file mode 100644 index 3acda62f58..0000000000 --- a/packages/kernel-node-runtime/src/io/socket-channel.ts +++ /dev/null @@ -1,191 +0,0 @@ -import type { IOChannel } from '@metamask/ocap-kernel'; -import fs from 'node:fs/promises'; -import * as net from 'node:net'; -import { StringDecoder } from 'node:string_decoder'; - -type PendingReader = { - resolve: (value: string | null) => void; -}; - -/** - * Create an IOChannel backed by a Unix domain socket. - * - * Creates a `net.Server` listening on the configured socket path. - * Accepts one connection at a time. Lines are `\n`-delimited. - * - * @param name - The channel name (for diagnostics). - * @param socketPath - The file path for the Unix domain socket. - * @returns A promise for the IOChannel, resolved once the server is listening. - */ -export async function makeSocketIOChannel( - name: string, - socketPath: string, -): Promise { - const lineQueue: string[] = []; - const readerQueue: PendingReader[] = []; - let currentSocket: net.Socket | null = null; - let decoder = new StringDecoder('utf8'); - let buffer = ''; - let closed = false; - - /** - * Deliver a line to a pending reader or enqueue it. - * - * @param line - The line to deliver. - */ - function deliverLine(line: string): void { - const reader = readerQueue.shift(); - if (reader) { - reader.resolve(line); - } else { - lineQueue.push(line); - } - } - - /** - * Handle the end of the input stream. - */ - function deliverEOF(): void { - while (readerQueue.length > 0) { - const reader = readerQueue.shift(); - reader?.resolve(null); - } - } - - /** - * Handle incoming data by splitting on newlines. - * - * @param data - The raw data buffer from the socket. - */ - function handleData(data: Buffer): void { - buffer += decoder.write(data); - let newlineIndex = buffer.indexOf('\n'); - while (newlineIndex !== -1) { - const line = buffer.slice(0, newlineIndex); - buffer = buffer.slice(newlineIndex + 1); - deliverLine(line); - newlineIndex = buffer.indexOf('\n'); - } - } - - /** - * Handle the channel disconnecting. - * - * @param socket - The socket that disconnected. - */ - function handleDisconnect(socket: net.Socket): void { - if (currentSocket !== socket) { - return; - } - // Flush any incomplete multi-byte sequence from the decoder - buffer += decoder.end(); - // Deliver any remaining buffered data as a final line - if (buffer.length > 0) { - deliverLine(buffer); - buffer = ''; - } - currentSocket = null; - deliverEOF(); - } - - const server = net.createServer((socket) => { - if (currentSocket) { - if (currentSocket.readableEnded || currentSocket.destroyed) { - // Old connection is dead but events haven't been fully processed; - // clean it up and accept the new connection. - currentSocket.removeAllListeners(); - currentSocket.destroy(); - currentSocket = null; - } else { - // Existing active client — reject the new connection - socket.destroy(); - return; - } - } - // Drain stale data from any previous connection, but keep pending - // readers alive so they can receive data from the new connection. - lineQueue.length = 0; - - currentSocket = socket; - decoder = new StringDecoder('utf8'); - buffer = ''; - - socket.on('data', handleData); - socket.on('end', () => handleDisconnect(socket)); - socket.on('error', () => handleDisconnect(socket)); - socket.on('close', () => handleDisconnect(socket)); - }); - - // Remove stale socket file if it exists - try { - await fs.unlink(socketPath); - } catch { - // Ignore if it doesn't exist - } - - await new Promise((resolve, reject) => { - server.on('error', reject); - server.listen(socketPath, () => { - server.removeListener('error', reject); - resolve(); - }); - }); - - const channel: IOChannel = { - async read(): Promise { - if (closed) { - return null; - } - const queued = lineQueue.shift(); - if (queued !== undefined) { - return queued; - } - // Block until data arrives (from a current or future client connection) - return new Promise((resolve) => { - readerQueue.push({ resolve }); - }); - }, - - async write(data: string): Promise { - if (closed) { - throw new Error(`IO channel "${name}" is closed`); - } - if (!currentSocket) { - throw new Error(`IO channel "${name}" has no connected client`); - } - const socket = currentSocket; - return new Promise((resolve, reject) => { - socket.write(`${data}\n`, (error) => { - if (error) { - reject(error); - } else { - resolve(); - } - }); - }); - }, - - async close(): Promise { - if (closed) { - return; - } - closed = true; - deliverEOF(); - if (currentSocket) { - currentSocket.destroy(); - currentSocket = null; - } - await new Promise((resolve) => { - server.close(() => resolve()); - }); - // Clean up socket file - try { - await fs.unlink(socketPath); - } catch { - // Ignore - } - }, - }; - - return channel; -} diff --git a/packages/kernel-node-runtime/src/io/socket-listener.test.ts b/packages/kernel-node-runtime/src/io/socket-listener.test.ts new file mode 100644 index 0000000000..68363158ea --- /dev/null +++ b/packages/kernel-node-runtime/src/io/socket-listener.test.ts @@ -0,0 +1,401 @@ +import type { IOChannel, IOListener } from '@metamask/ocap-kernel'; +import fs from 'node:fs/promises'; +import * as net from 'node:net'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { describe, it, expect, afterEach } from 'vitest'; + +import { makeSocketIOListener } from './socket-listener.ts'; + +function tempSocketPath(): string { + return path.join( + os.tmpdir(), + `io-test-${Date.now()}-${Math.random().toString(36).slice(2)}.sock`, + ); +} + +async function connectToSocket(socketPath: string): Promise { + return new Promise((resolve, reject) => { + const client = net.createConnection(socketPath, () => { + client.removeListener('error', reject); + resolve(client); + }); + client.on('error', reject); + }); +} + +async function writeLine(socket: net.Socket, line: string): Promise { + return new Promise((resolve, reject) => { + socket.write(`${line}\n`, (error) => { + if (error) { + reject(error); + } else { + resolve(); + } + }); + }); +} + +async function readLine(socket: net.Socket): Promise { + return new Promise((resolve) => { + let buffer = ''; + const onData = (data: Buffer): void => { + buffer += data.toString(); + const idx = buffer.indexOf('\n'); + if (idx !== -1) { + socket.removeListener('data', onData); + resolve(buffer.slice(0, idx)); + } + }; + socket.on('data', onData); + }); +} + +async function fileExists(filePath: string): Promise { + try { + await fs.access(filePath); + return true; + } catch { + return false; + } +} + +const settle = async (ms = 20): Promise => + new Promise((resolve) => setTimeout(resolve, ms)); + +describe('makeSocketIOListener', () => { + const listeners: IOListener[] = []; + const clients: net.Socket[] = []; + + afterEach(async () => { + for (const client of clients) { + client.destroy(); + } + clients.length = 0; + for (const listener of listeners) { + await listener.close(); + } + listeners.length = 0; + }); + + /** + * Create a listener that is torn down after the test. + * + * @param socketPath - Path for the Unix domain socket. + * @returns The listener. + */ + async function makeTracked(socketPath: string): Promise { + const listener = await makeSocketIOListener('test', socketPath); + listeners.push(listener); + return listener; + } + + /** + * Connect a client that is destroyed after the test. + * + * @param socketPath - Path for the Unix domain socket. + * @returns The connected client socket. + */ + async function connectTracked(socketPath: string): Promise { + const client = await connectToSocket(socketPath); + clients.push(client); + return client; + } + + it('creates a listening socket', async () => { + const socketPath = tempSocketPath(); + await makeTracked(socketPath); + + expect(await fileExists(socketPath)).toBe(true); + }); + + describe('accept()', () => { + it('yields a channel for a connecting peer', async () => { + const socketPath = tempSocketPath(); + const listener = await makeTracked(socketPath); + + const acceptPromise = listener.accept(); + const client = await connectTracked(socketPath); + await writeLine(client, 'hello'); + + const channel = await acceptPromise; + expect(await channel?.read()).toBe('hello'); + }); + + it('queues peers that connect before accept is called', async () => { + const socketPath = tempSocketPath(); + const listener = await makeTracked(socketPath); + + const client = await connectTracked(socketPath); + await writeLine(client, 'early'); + await settle(); + + const channel = await listener.accept(); + expect(await channel?.read()).toBe('early'); + }); + + it('yields one channel per peer, in connection order', async () => { + const socketPath = tempSocketPath(); + const listener = await makeTracked(socketPath); + + const first = await connectTracked(socketPath); + await settle(); + const second = await connectTracked(socketPath); + await settle(); + + const channelA = await listener.accept(); + const channelB = await listener.accept(); + await writeLine(first, 'from-first'); + await writeLine(second, 'from-second'); + + expect(await channelA?.read()).toBe('from-first'); + expect(await channelB?.read()).toBe('from-second'); + }); + + it('returns null once the listener is closed', async () => { + const socketPath = tempSocketPath(); + const listener = await makeSocketIOListener('test', socketPath); + + await listener.close(); + + expect(await listener.accept()).toBeNull(); + }); + + it('releases a pending accept when the listener closes', async () => { + const socketPath = tempSocketPath(); + const listener = await makeSocketIOListener('test', socketPath); + + const acceptPromise = listener.accept(); + await listener.close(); + + expect(await acceptPromise).toBeNull(); + }); + }); + + describe('concurrent connections', () => { + it('serves several peers at once without mixing their data', async () => { + const socketPath = tempSocketPath(); + const listener = await makeTracked(socketPath); + + const alice = await connectTracked(socketPath); + await settle(); + const bob = await connectTracked(socketPath); + await settle(); + + const aliceChannel = (await listener.accept()) as IOChannel; + const bobChannel = (await listener.accept()) as IOChannel; + + // Interleave traffic from both peers. + await writeLine(alice, 'alice-1'); + await writeLine(bob, 'bob-1'); + await writeLine(alice, 'alice-2'); + await writeLine(bob, 'bob-2'); + await settle(); + + expect(await aliceChannel.read()).toBe('alice-1'); + expect(await aliceChannel.read()).toBe('alice-2'); + expect(await bobChannel.read()).toBe('bob-1'); + expect(await bobChannel.read()).toBe('bob-2'); + }); + + it('routes each write back to its own peer', async () => { + const socketPath = tempSocketPath(); + const listener = await makeTracked(socketPath); + + const alice = await connectTracked(socketPath); + await settle(); + const bob = await connectTracked(socketPath); + await settle(); + + const aliceChannel = (await listener.accept()) as IOChannel; + const bobChannel = (await listener.accept()) as IOChannel; + + const aliceHeard = readLine(alice); + const bobHeard = readLine(bob); + await aliceChannel.write('for-alice'); + await bobChannel.write('for-bob'); + + expect(await aliceHeard).toBe('for-alice'); + expect(await bobHeard).toBe('for-bob'); + }); + + it('leaves one peer unaffected when another disconnects', async () => { + const socketPath = tempSocketPath(); + const listener = await makeTracked(socketPath); + + const doomed = await connectToSocket(socketPath); + await settle(); + const survivor = await connectTracked(socketPath); + await settle(); + + const doomedChannel = (await listener.accept()) as IOChannel; + const survivorChannel = (await listener.accept()) as IOChannel; + + doomed.destroy(); + await settle(); + + expect(await doomedChannel.read()).toBeNull(); + await writeLine(survivor, 'still-here'); + expect(await survivorChannel.read()).toBe('still-here'); + }); + }); + + describe('connection channels', () => { + it('writes lines to its peer', async () => { + const socketPath = tempSocketPath(); + const listener = await makeTracked(socketPath); + + const client = await connectTracked(socketPath); + const channel = (await listener.accept()) as IOChannel; + + const linePromise = readLine(client); + await channel.write('output'); + + expect(await linePromise).toBe('output'); + }); + + it('queues lines that arrive before read is called', async () => { + const socketPath = tempSocketPath(); + const listener = await makeTracked(socketPath); + + const client = await connectTracked(socketPath); + const channel = (await listener.accept()) as IOChannel; + + await writeLine(client, 'a'); + await writeLine(client, 'b'); + await settle(); + + expect(await channel.read()).toBe('a'); + expect(await channel.read()).toBe('b'); + }); + + it('returns null to a pending read when the peer disconnects', async () => { + const socketPath = tempSocketPath(); + const listener = await makeTracked(socketPath); + + const client = await connectToSocket(socketPath); + const channel = (await listener.accept()) as IOChannel; + + const readPromise = channel.read(); + client.destroy(); + + expect(await readPromise).toBeNull(); + }); + + it('delivers buffered lines before reporting EOF', async () => { + const socketPath = tempSocketPath(); + const listener = await makeTracked(socketPath); + + const client = await connectToSocket(socketPath); + const channel = (await listener.accept()) as IOChannel; + + await writeLine(client, 'last-words'); + await settle(); + client.destroy(); + await settle(); + + // Data the peer sent before going away is not lost. + expect(await channel.read()).toBe('last-words'); + expect(await channel.read()).toBeNull(); + }); + + it('returns null after the channel is closed', async () => { + const socketPath = tempSocketPath(); + const listener = await makeTracked(socketPath); + + await connectTracked(socketPath); + const channel = (await listener.accept()) as IOChannel; + await channel.close(); + + expect(await channel.read()).toBeNull(); + }); + + it('throws on write after the channel is closed', async () => { + const socketPath = tempSocketPath(); + const listener = await makeTracked(socketPath); + + await connectTracked(socketPath); + const channel = (await listener.accept()) as IOChannel; + await channel.close(); + + await expect(channel.write('data')).rejects.toThrow('is closed'); + }); + + it('handles multi-byte UTF-8 split across TCP chunks', async () => { + const socketPath = tempSocketPath(); + const listener = await makeTracked(socketPath); + + const client = await connectTracked(socketPath); + const channel = (await listener.accept()) as IOChannel; + + // U+1F600 (😀) is 4 bytes: f0 9f 98 80 + const emoji = '\u{1F600}'; + const encoded = Buffer.from(`hello ${emoji} world\n`, 'utf8'); + + // Split in the middle of the emoji (after 2 of its 4 bytes) + const splitPoint = Buffer.from('hello ', 'utf8').length + 2; + + await new Promise((resolve, reject) => { + client.write(encoded.subarray(0, splitPoint), (error) => + error ? reject(error) : resolve(), + ); + }); + await settle(10); + await new Promise((resolve, reject) => { + client.write(encoded.subarray(splitPoint), (error) => + error ? reject(error) : resolve(), + ); + }); + await settle(10); + + expect(await channel.read()).toBe(`hello ${emoji} world`); + }); + }); + + describe('close()', () => { + it('cleans up the socket file', async () => { + const socketPath = tempSocketPath(); + const listener = await makeSocketIOListener('test', socketPath); + + expect(await fileExists(socketPath)).toBe(true); + await listener.close(); + expect(await fileExists(socketPath)).toBe(false); + }); + + it('closes the connections it handed out', async () => { + const socketPath = tempSocketPath(); + const listener = await makeSocketIOListener('test', socketPath); + + await connectTracked(socketPath); + const channel = (await listener.accept()) as IOChannel; + + await listener.close(); + + expect(await channel.read()).toBeNull(); + await expect(channel.write('data')).rejects.toThrow('is closed'); + }); + + it('is idempotent', async () => { + const socketPath = tempSocketPath(); + const listener = await makeSocketIOListener('test', socketPath); + + await listener.close(); + expect(await listener.close()).toBeUndefined(); + }); + }); + + it('removes a stale socket file on creation', async () => { + const socketPath = tempSocketPath(); + + const first = await makeSocketIOListener('test', socketPath); + await first.close(); + + // Recreate a stale file + await fs.writeFile(socketPath, ''); + + // Should succeed despite the stale file + await makeTracked(socketPath); + + expect(await fileExists(socketPath)).toBe(true); + }); +}); diff --git a/packages/kernel-node-runtime/src/io/socket-listener.ts b/packages/kernel-node-runtime/src/io/socket-listener.ts new file mode 100644 index 0000000000..48beefa972 --- /dev/null +++ b/packages/kernel-node-runtime/src/io/socket-listener.ts @@ -0,0 +1,253 @@ +import type { IOChannel, IOListener } from '@metamask/ocap-kernel'; +import fs from 'node:fs/promises'; +import * as net from 'node:net'; +import { StringDecoder } from 'node:string_decoder'; + +type PendingReader = { + resolve: (value: string | null) => void; +}; + +type PendingAcceptor = { + resolve: (value: IOChannel | null) => void; +}; + +/** + * Wrap a connected socket as an `IOChannel`. + * + * All of the channel's state — receive buffer, decoder, queued lines, and + * pending readers — is local to this function, so concurrent connections + * cannot interfere with one another. This is the reason the listener can + * serve many peers at once where a single shared channel could not. + * + * @param name - The connection name (for diagnostics). + * @param socket - The connected socket. + * @param onClosed - Invoked once when the connection is finished, whether + * because the peer went away or because `close()` was called. + * @returns The channel for this connection. + */ +function makeConnectionChannel( + name: string, + socket: net.Socket, + onClosed: () => void, +): IOChannel { + const lineQueue: string[] = []; + const readerQueue: PendingReader[] = []; + const decoder = new StringDecoder('utf8'); + let buffer = ''; + let ended = false; + let closed = false; + + /** + * Deliver a line to a pending reader or enqueue it for a future read. + * + * @param line - The line to deliver. + */ + function deliverLine(line: string): void { + const reader = readerQueue.shift(); + if (reader) { + reader.resolve(line); + } else { + lineQueue.push(line); + } + } + + /** + * Resolve every waiting reader with EOF. + */ + function deliverEOF(): void { + while (readerQueue.length > 0) { + readerQueue.shift()?.resolve(null); + } + } + + /** + * Split incoming bytes into `\n`-delimited lines. + * + * @param data - The raw data from the socket. + */ + function handleData(data: Buffer): void { + buffer += decoder.write(data); + let newlineIndex = buffer.indexOf('\n'); + while (newlineIndex !== -1) { + const line = buffer.slice(0, newlineIndex); + buffer = buffer.slice(newlineIndex + 1); + deliverLine(line); + newlineIndex = buffer.indexOf('\n'); + } + } + + /** + * Handle the peer going away. Flushes any trailing partial line, then + * reports EOF. Unlike a shared channel, there is no ambiguity about + * whose session ended: this channel serves exactly one peer, so the + * end of the socket is the end of the channel. + */ + function handleEnd(): void { + if (ended) { + return; + } + ended = true; + buffer += decoder.end(); + if (buffer.length > 0) { + deliverLine(buffer); + buffer = ''; + } + deliverEOF(); + onClosed(); + } + + socket.on('data', handleData); + socket.on('end', handleEnd); + socket.on('error', handleEnd); + socket.on('close', handleEnd); + + return { + async read(): Promise { + const queued = lineQueue.shift(); + if (queued !== undefined) { + return queued; + } + if (ended || closed) { + return null; + } + return new Promise((resolve) => { + readerQueue.push({ resolve }); + }); + }, + + async write(data: string): Promise { + if (closed || ended) { + throw new Error(`IO connection "${name}" is closed`); + } + return new Promise((resolve, reject) => { + socket.write(`${data}\n`, (error) => { + if (error) { + reject(error); + } else { + resolve(); + } + }); + }); + }, + + async close(): Promise { + if (closed) { + return; + } + closed = true; + deliverEOF(); + socket.destroy(); + // `close` on the socket will fire handleEnd, but call it directly so + // the caller's `onClosed` bookkeeping is done by the time close() + // resolves rather than a turn later. + handleEnd(); + }, + }; +} + +/** + * Create an `IOListener` backed by a Unix domain socket. + * + * Creates a `net.Server` on the configured path. Every connection that + * arrives becomes its own `IOChannel`, handed out by `accept()`, so any + * number of peers can be served concurrently. Lines are `\n`-delimited. + * + * Connections that arrive before anyone calls `accept()` are queued, so a + * peer connecting during startup is not dropped. + * + * @param name - The listener name (for diagnostics). + * @param socketPath - The file path for the Unix domain socket. + * @returns A promise for the IOListener, resolved once the server is listening. + */ +export async function makeSocketIOListener( + name: string, + socketPath: string, +): Promise { + /** Connections that have arrived but not yet been accepted. */ + const readyQueue: IOChannel[] = []; + /** Callers waiting in `accept()` for a connection to arrive. */ + const acceptorQueue: PendingAcceptor[] = []; + /** Live connections, so `close()` can tear them all down. */ + const liveChannels = new Set(); + let closed = false; + let nextConnectionId = 0; + + const server = net.createServer((socket) => { + if (closed) { + socket.destroy(); + return; + } + nextConnectionId += 1; + const connectionName = `${name}:${nextConnectionId}`; + const channel: IOChannel = makeConnectionChannel( + connectionName, + socket, + () => { + liveChannels.delete(channel); + }, + ); + liveChannels.add(channel); + + const acceptor = acceptorQueue.shift(); + if (acceptor) { + acceptor.resolve(channel); + } else { + readyQueue.push(channel); + } + }); + + // Remove stale socket file if it exists + try { + await fs.unlink(socketPath); + } catch { + // Ignore if it doesn't exist + } + + await new Promise((resolve, reject) => { + server.on('error', reject); + server.listen(socketPath, () => { + server.removeListener('error', reject); + resolve(); + }); + }); + + return { + async accept(): Promise { + const ready = readyQueue.shift(); + if (ready) { + return ready; + } + if (closed) { + return null; + } + return new Promise((resolve) => { + acceptorQueue.push({ resolve }); + }); + }, + + async close(): Promise { + if (closed) { + return; + } + closed = true; + // Release anyone parked in accept() so their loops can exit. + while (acceptorQueue.length > 0) { + acceptorQueue.shift()?.resolve(null); + } + readyQueue.length = 0; + for (const channel of [...liveChannels]) { + await channel.close(); + } + liveChannels.clear(); + await new Promise((resolve) => { + server.close(() => resolve()); + }); + // Clean up socket file + try { + await fs.unlink(socketPath); + } catch { + // Ignore + } + }, + }; +} diff --git a/packages/kernel-node-runtime/src/kernel/make-kernel.ts b/packages/kernel-node-runtime/src/kernel/make-kernel.ts index 81e4a37e43..27803916d8 100644 --- a/packages/kernel-node-runtime/src/kernel/make-kernel.ts +++ b/packages/kernel-node-runtime/src/kernel/make-kernel.ts @@ -3,12 +3,12 @@ import { makeSQLKernelDatabase } from '@metamask/kernel-store/sqlite/nodejs'; import { Logger } from '@metamask/logger'; import { Kernel } from '@metamask/ocap-kernel'; import type { - IOChannelFactory, + IOListenerFactory, SystemSubclusterConfig, } from '@metamask/ocap-kernel'; import { NodejsPlatformServices } from './PlatformServices.ts'; -import { makeIOChannelFactory } from '../io/index.ts'; +import { makeIOListenerFactory } from '../io/index.ts'; /** * Result of {@link makeKernel}. @@ -27,7 +27,7 @@ export type MakeKernelResult = { * @param options.dbFilename - The filename of the SQLite database file. * @param options.logger - The logger to use for the kernel. * @param options.keySeed - Optional seed for libp2p key generation. - * @param options.ioChannelFactory - Optional factory for creating IO channels. + * @param options.ioListenerFactory - Optional factory for creating IO listeners. * @param options.systemSubclusters - Optional system subcluster configurations. * @returns The kernel and its database. */ @@ -37,7 +37,7 @@ export async function makeKernel({ dbFilename, logger, keySeed, - ioChannelFactory, + ioListenerFactory, systemSubclusters, }: { workerFilePath?: string; @@ -45,7 +45,7 @@ export async function makeKernel({ dbFilename?: string; logger?: Logger; keySeed?: string | undefined; - ioChannelFactory?: IOChannelFactory; + ioListenerFactory?: IOListenerFactory; systemSubclusters?: SystemSubclusterConfig[]; }): Promise { const rootLogger = logger ?? new Logger('kernel-worker'); @@ -62,7 +62,7 @@ export async function makeKernel({ resetStorage, logger: rootLogger.subLogger({ tags: ['kernel'] }), keySeed, - ioChannelFactory: ioChannelFactory ?? makeIOChannelFactory(), + ioListenerFactory: ioListenerFactory ?? makeIOListenerFactory(), ...(systemSubclusters ? { systemSubclusters } : {}), }); diff --git a/packages/kernel-node-runtime/test/helpers/kernel.ts b/packages/kernel-node-runtime/test/helpers/kernel.ts index 1595897c12..313604f830 100644 --- a/packages/kernel-node-runtime/test/helpers/kernel.ts +++ b/packages/kernel-node-runtime/test/helpers/kernel.ts @@ -4,7 +4,7 @@ import { Logger } from '@metamask/logger'; import { Kernel, kunser } from '@metamask/ocap-kernel'; import type { ClusterConfig, - IOChannelFactory, + IOListenerFactory, SystemSubclusterConfig, } from '@metamask/ocap-kernel'; @@ -14,7 +14,7 @@ type MakeTestKernelOptions = { resetStorage?: boolean; mnemonic?: string; systemSubclusters?: SystemSubclusterConfig[]; - ioChannelFactory?: IOChannelFactory; + ioListenerFactory?: IOListenerFactory; }; /** @@ -26,7 +26,7 @@ type MakeTestKernelOptions = { * @param options.resetStorage - Whether to reset the storage (default: true). * @param options.mnemonic - Optional BIP39 mnemonic string. * @param options.systemSubclusters - Optional system subcluster configurations. - * @param options.ioChannelFactory - Optional IO channel factory. + * @param options.ioListenerFactory - Optional IO listener factory. * @returns The kernel. */ export async function makeTestKernel( @@ -37,7 +37,7 @@ export async function makeTestKernel( resetStorage = true, mnemonic, systemSubclusters, - ioChannelFactory, + ioListenerFactory, } = options; const logger = new Logger('test-kernel'); @@ -48,7 +48,7 @@ export async function makeTestKernel( resetStorage, mnemonic, systemSubclusters, - ioChannelFactory, + ioListenerFactory, logger: logger.subLogger({ tags: ['kernel'] }), }); From e1eaefbc896ac5d09a2ed54e4f18083d5585189a Mon Sep 17 00:00:00 2001 From: Chip Morningstar Date: Tue, 4 Aug 2026 16:14:03 -0700 Subject: [PATCH 05/24] test(kernel-test): io-vat accepts connections; cover two concurrent peers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The io-vat's `repl` endowment is now an IOListener, so it accepts connections and addresses them by index, letting a test drive several peers independently. The integration test drops its hand-rolled duplicate channel in favour of the real makeIOListenerFactory, and adds a case covering two concurrent peers end to end through a real kernel — neither reading the other's data nor receiving the other's writes. That case was unrepresentable before: the second connection was destroyed on arrival. Co-Authored-By: Claude Opus 4.7 --- packages/kernel-test/src/io.test.ts | 273 ++++++++++-------------- packages/kernel-test/src/vats/io-vat.ts | 40 +++- 2 files changed, 139 insertions(+), 174 deletions(-) diff --git a/packages/kernel-test/src/io.test.ts b/packages/kernel-test/src/io.test.ts index 62ccc7f5a7..df94edb03a 100644 --- a/packages/kernel-test/src/io.test.ts +++ b/packages/kernel-test/src/io.test.ts @@ -1,7 +1,6 @@ import { makeSQLKernelDatabase } from '@metamask/kernel-store/sqlite/nodejs'; import { waitUntilQuiescent } from '@metamask/kernel-utils'; import { Kernel } from '@metamask/ocap-kernel'; -import type { IOChannel, IOConfig } from '@metamask/ocap-kernel'; import * as net from 'node:net'; import * as os from 'node:os'; import * as path from 'node:path'; @@ -53,125 +52,56 @@ async function readLine(socket: net.Socket): Promise { }); } -async function makeTestSocketChannel( - _name: string, +/** + * Stand up a kernel wired to the real Node listener factory, with an + * io-vat subcluster listening on `socketPath`. + * + * @param socketPath - Path for the listener's Unix domain socket. + * @returns The kernel and the io-vat's root kref. + */ +async function makeIoKernel( socketPath: string, -): Promise { - const fsPromises = await import('node:fs/promises'); - const lineQueue: string[] = []; - const readerQueue: { resolve: (value: string | null) => void }[] = []; - let currentSocket: net.Socket | null = null; - let lineBuffer = ''; - let closed = false; - - function deliverLine(line: string): void { - const reader = readerQueue.shift(); - if (reader) { - reader.resolve(line); - } else { - lineQueue.push(line); - } - } - - function deliverEOF(): void { - while (readerQueue.length > 0) { - readerQueue.shift()?.resolve(null); - } - } - - const server = net.createServer((socket) => { - if (currentSocket) { - socket.destroy(); - return; - } - currentSocket = socket; - lineBuffer = ''; - socket.on('data', (data: Buffer) => { - lineBuffer += data.toString(); - let idx = lineBuffer.indexOf('\n'); - while (idx !== -1) { - deliverLine(lineBuffer.slice(0, idx)); - lineBuffer = lineBuffer.slice(idx + 1); - idx = lineBuffer.indexOf('\n'); - } - }); - socket.on('end', () => { - if (lineBuffer.length > 0) { - deliverLine(lineBuffer); - lineBuffer = ''; - } - currentSocket = null; - deliverEOF(); - }); - socket.on('error', () => { - currentSocket = null; - deliverEOF(); - }); - }); - - try { - await fsPromises.unlink(socketPath); - } catch { - // ignore - } - - await new Promise((resolve, reject) => { - server.on('error', reject); - server.listen(socketPath, () => { - server.removeListener('error', reject); - resolve(); - }); +): Promise<{ kernel: Kernel; rootKref: string }> { + const kernelDatabase = await makeSQLKernelDatabase({ + dbFilename: ':memory:', }); + const { logger } = makeTestLogger(); - return { - async read() { - if (closed) { - return null; - } - const queued = lineQueue.shift(); - if (queued !== undefined) { - return queued; - } - if (!currentSocket) { - return null; - } - return new Promise((resolve) => { - readerQueue.push({ resolve }); - }); + const { NodejsPlatformServices, makeIOListenerFactory } = await import( + '@metamask/kernel-node-runtime' + ); + const kernel = await Kernel.make( + new NodejsPlatformServices({ + logger: logger.subLogger({ tags: ['platform'] }), + }), + kernelDatabase, + { + resetStorage: true, + logger, + ioListenerFactory: makeIOListenerFactory(), }, - async write(data: string) { - if (!currentSocket) { - throw new Error('no connected client'); - } - const socket = currentSocket; - return new Promise((resolve, reject) => { - socket.write(`${data}\n`, (error) => { - if (error) { - reject(error); - } else { - resolve(); - } - }); - }); + ); + + const { rootKref } = await kernel.launchSubcluster({ + bootstrap: 'io', + forceReset: true, + io: { + repl: { + type: 'socket' as const, + path: socketPath, + }, }, - async close() { - if (closed) { - return; - } - closed = true; - deliverEOF(); - currentSocket?.destroy(); - currentSocket = null; - await new Promise((resolve) => { - server.close(() => resolve()); - }); - try { - await fsPromises.unlink(socketPath); - } catch { - // ignore - } + services: ['repl'], + vats: { + io: { + bundleSpec: getBundleSpec('io-vat'), + parameters: { name: 'io' }, + }, }, - }; + }); + await waitUntilQuiescent(); + + return { kernel, rootKref }; } describe('IO kernel service', () => { @@ -184,65 +114,20 @@ describe('IO kernel service', () => { clients.length = 0; }); - it('reads and writes through an IO channel', async () => { + it('reads and writes through an accepted connection', async () => { const socketPath = tempSocketPath(); - const kernelDatabase = await makeSQLKernelDatabase({ - dbFilename: ':memory:', - }); - const { logger } = makeTestLogger(); + const { kernel, rootKref } = await makeIoKernel(socketPath); - const { NodejsPlatformServices } = await import( - '@metamask/kernel-node-runtime' - ); - const kernel = await Kernel.make( - new NodejsPlatformServices({ - logger: logger.subLogger({ tags: ['platform'] }), - }), - kernelDatabase, - { - resetStorage: true, - logger, - ioChannelFactory: async (name: string, config: IOConfig) => { - if (config.type !== 'socket') { - throw new Error(`unsupported: ${config.type}`); - } - return makeTestSocketChannel(name, config.path); - }, - }, - ); - - const config = { - bootstrap: 'io', - forceReset: true, - io: { - repl: { - type: 'socket' as const, - path: socketPath, - }, - }, - services: ['repl'], - vats: { - io: { - bundleSpec: getBundleSpec('io-vat'), - parameters: { name: 'io' }, - }, - }, - }; - - const { rootKref } = await kernel.launchSubcluster(config); - await waitUntilQuiescent(); - - // Connect to the socket const client = await connectToSocket(socketPath); clients.push(client); - - // Small delay for connection setup await new Promise((resolve) => setTimeout(resolve, 20)); + await kernel.queueMessage(rootKref, 'doAccept', []); + await waitUntilQuiescent(100); + // Send a line from the test to the vat await writeLine(client, 'hello from test'); - // Trigger the vat to read and verify it received the data await kernel.queueMessage(rootKref, 'doRead', []); await waitUntilQuiescent(100); @@ -259,7 +144,63 @@ describe('IO kernel service', () => { await kernel.queueMessage(rootKref, 'doWrite', ['hello from vat']); await waitUntilQuiescent(100); - const received = await linePromise; - expect(received).toBe('hello from vat'); + expect(await linePromise).toBe('hello from vat'); + }); + + it('serves two concurrent peers without crossing their traffic', async () => { + const socketPath = tempSocketPath(); + const { kernel, rootKref } = await makeIoKernel(socketPath); + + const alice = await connectToSocket(socketPath); + clients.push(alice); + await new Promise((resolve) => setTimeout(resolve, 20)); + const bob = await connectToSocket(socketPath); + clients.push(bob); + await new Promise((resolve) => setTimeout(resolve, 20)); + + // Both peers are accepted — under the old single-client channel the + // second connection would have been destroyed outright. + await kernel.queueMessage(rootKref, 'doAccept', []); + await waitUntilQuiescent(100); + await kernel.queueMessage(rootKref, 'doAccept', []); + await waitUntilQuiescent(100); + + const countResult = await kernel.queueMessage( + rootKref, + 'getConnectionCount', + [], + ); + await waitUntilQuiescent(100); + expect(countResult.body).toContain('2'); + + // Each peer's line arrives on its own connection. + await writeLine(alice, 'from-alice'); + await writeLine(bob, 'from-bob'); + await new Promise((resolve) => setTimeout(resolve, 20)); + + await kernel.queueMessage(rootKref, 'doRead', [0]); + await waitUntilQuiescent(100); + await kernel.queueMessage(rootKref, 'doRead', [1]); + await waitUntilQuiescent(100); + + const bufferResult = await kernel.queueMessage( + rootKref, + 'getReadBuffer', + [], + ); + await waitUntilQuiescent(100); + expect(bufferResult.body).toContain('from-alice'); + expect(bufferResult.body).toContain('from-bob'); + + // And each write goes back to the right peer. + const aliceHeard = readLine(alice); + const bobHeard = readLine(bob); + await kernel.queueMessage(rootKref, 'doWrite', ['for-alice', 0]); + await waitUntilQuiescent(100); + await kernel.queueMessage(rootKref, 'doWrite', ['for-bob', 1]); + await waitUntilQuiescent(100); + + expect(await aliceHeard).toBe('for-alice'); + expect(await bobHeard).toBe('for-bob'); }); }); diff --git a/packages/kernel-test/src/vats/io-vat.ts b/packages/kernel-test/src/vats/io-vat.ts index 04b582fde1..0be770d45e 100644 --- a/packages/kernel-test/src/vats/io-vat.ts +++ b/packages/kernel-test/src/vats/io-vat.ts @@ -7,6 +7,10 @@ import type { TestPowers } from '../test-powers.ts'; /** * Build function for testing IO kernel services. * + * The `repl` endowment is an `IOListener`, so the vat accepts connections + * from it and keeps each one separately. `doRead`/`doWrite` name a + * connection by index so a test can drive several peers independently. + * * @param vatPowers - Special powers granted to this vat. * @param parameters - Initialization parameters from the vat's config object. * @param parameters.name - The name of the vat. @@ -19,26 +23,46 @@ export function buildRootObject( ) { const name = parameters?.name ?? 'io-vat'; const tlog = unwrapTestLogger(vatPowers, name); - let ioService: unknown; + let listener: unknown; + const connections: unknown[] = []; const readBuffer: string[] = []; return makeDefaultExo('root', { async bootstrap(_vats: unknown, services: { repl: unknown }) { tlog('bootstrap'); - ioService = services.repl; + listener = services.repl; + }, + /** + * Accept the next waiting connection, appending it to the list. + * + * @returns The index of the accepted connection, or -1 on EOF. + */ + async doAccept() { + const connection = await E(listener).accept(); + if (!connection) { + tlog('accept: listener closed'); + return -1; + } + connections.push(connection); + const index = connections.length - 1; + tlog(`accepted connection ${index}`); + return index; }, - async doRead() { - const line = await E(ioService).read(); - tlog(`read: ${line}`); + async doRead(index = 0) { + const line = await E(connections[index]).read(); + tlog(`read[${index}]: ${line}`); readBuffer.push(String(line)); return line; }, - async doWrite(data: string) { - await E(ioService).write(data); - tlog(`wrote: ${data}`); + async doWrite(data: string, index = 0) { + await E(connections[index]).write(data); + tlog(`wrote[${index}]: ${data}`); }, async getReadBuffer() { return [...readBuffer]; }, + async getConnectionCount() { + return connections.length; + }, }); } From 4c338c6524282d768662716834698971bba19cb6 Mon Sep 17 00:00:00 2001 From: Chip Morningstar Date: Tue, 4 Aug 2026 16:14:12 -0700 Subject: [PATCH 06/24] feat(kernel-utils,service-discovery-types): interface variant for JsonSchema MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds an `InterfaceJsonSchema` variant — `{ type: 'interface', description?, methods }` — describing an object whose methods can be invoked, so a method that hands back an object reference can declare the returned object's API inline and a client need not make a second round-trip to discover it. The `methods` field is recursive, so a returned interface can itself return interfaces. The schema describes an *interface*. Whether the reference to that object is unforgeable is a property of the reference plumbing, not of the description, so the same schema serves either case. service-discovery-types converts the new variant to a `RemotableSpec` via `interfaceJsonSchemaToRemotableSpec`, which means `remotable` is no longer among the kinds `JsonSchema` cannot express. Co-Authored-By: Claude Opus 4.7 --- .../kernel-utils/src/discoverable.test.ts | 32 +++++++ .../kernel-utils/src/json-schema-to-struct.ts | 12 +++ packages/kernel-utils/src/schema.ts | 30 +++++- .../service-discovery-types/src/index.test.ts | 93 +++++++++++++++++++ .../src/method-schema-convert.ts | 33 ++++++- 5 files changed, 196 insertions(+), 4 deletions(-) diff --git a/packages/kernel-utils/src/discoverable.test.ts b/packages/kernel-utils/src/discoverable.test.ts index 3075c09b01..1a328368d7 100644 --- a/packages/kernel-utils/src/discoverable.test.ts +++ b/packages/kernel-utils/src/discoverable.test.ts @@ -143,6 +143,38 @@ describe('makeDiscoverableExo', () => { ); }); + it('accepts a return schema describing an interface', () => { + const factorySchema: MethodSchema = { + description: 'Return a counter object', + args: {}, + returns: { + type: 'interface', + description: 'A stateful counter', + methods: { + increment: { + description: 'Bump the counter and return the new value', + args: {}, + returns: { type: 'number', description: 'The new count' }, + }, + reset: { + description: 'Reset the counter to zero', + args: {}, + }, + }, + }, + }; + const methods = { makeCounter: () => ({}) }; + const schema: Record = { + makeCounter: factorySchema, + }; + + const exo = makeDiscoverableExo('CounterFactory', methods, schema); + + expect(exo[GET_DESCRIPTION]()).toStrictEqual({ + makeCounter: factorySchema, + }); + }); + it('re-throws errors from makeExo that are not about describe key', () => { const testError = new Error('Some other error from makeExo'); makeExoMock.mockImplementation(() => { diff --git a/packages/kernel-utils/src/json-schema-to-struct.ts b/packages/kernel-utils/src/json-schema-to-struct.ts index 944cb0dfe1..59d16ced92 100644 --- a/packages/kernel-utils/src/json-schema-to-struct.ts +++ b/packages/kernel-utils/src/json-schema-to-struct.ts @@ -87,6 +87,18 @@ export function jsonSchemaToStruct(schema: JsonSchema): Struct { } return looseObjectStruct(schema); } + case 'interface': { + // An interface reference: at runtime the value is an object (possibly + // an exo, remotable, or presence). We can't introspect its methods + // here — that's the receiver's responsibility on invocation. Validate + // that it's a non-null object and pass through. + return define('JsonSchemaInterface', (value) => { + if (typeof value !== 'object' || value === null) { + return 'Expected an object reference'; + } + return true; + }) as Struct; + } default: { const _never: never = schema; throw new TypeError(`Unsupported JSON schema: ${String(_never)}`); diff --git a/packages/kernel-utils/src/schema.ts b/packages/kernel-utils/src/schema.ts index b3534b8e13..6f79d3752e 100644 --- a/packages/kernel-utils/src/schema.ts +++ b/packages/kernel-utils/src/schema.ts @@ -1,11 +1,13 @@ /** - * JSON Schema type for describing values. Supports primitives, arrays, and objects + * JSON Schema type for describing values. Supports primitives, arrays, objects, + * and object interfaces (i.e. an object with methods you can invoke), * with recursive definitions. */ export type JsonSchema = | PrimitiveJsonSchema | ArrayJsonSchema - | ObjectJsonSchema; + | ObjectJsonSchema + | InterfaceJsonSchema; /** * Primitive JSON Schema types (string, number, boolean). @@ -37,6 +39,30 @@ type ObjectJsonSchema = { additionalProperties?: boolean; }; +/** + * Schema describing an object interface — a reference to an object whose + * methods can be invoked. Used as the return-type schema for methods that + * hand back an object reference (whether local or across a boundary), so + * a client can learn the returned object's API inline from the parent + * description without an extra round-trip. + * + * The `methods` field is recursive: any method here can itself return an + * interface, and so on. + * + * Naming note: this schema describes an object interface. Whether the + * reference to that object is unforgeable (i.e. an ocap in the strict + * sense) is a property of the reference plumbing (which vat holds it, + * whether it crossed a CapTP boundary, etc.), not of the interface + * description itself. Same schema either way. + */ +type InterfaceJsonSchema = { + type: 'interface'; + description?: string; + methods: { + [key: string]: MethodSchema; + }; +}; + /** * Schema describing a method, including its purpose, arguments, and return value. */ diff --git a/packages/service-discovery-types/src/index.test.ts b/packages/service-discovery-types/src/index.test.ts index 3f2402ca27..fd876ed6af 100644 --- a/packages/service-discovery-types/src/index.test.ts +++ b/packages/service-discovery-types/src/index.test.ts @@ -308,4 +308,97 @@ describe('methodsToRemotableSpec', () => { }, }); }); + + it('translates interface-typed returns into RemotableTypeSpec', () => { + const result: RemotableSpec = methodsToRemotableSpec({ + methods: { + makeCounter: { + description: 'make a counter', + args: {}, + returns: { + type: 'interface', + description: 'a stateful counter', + methods: { + increment: { + description: 'bump and return', + args: {}, + returns: { type: 'number' }, + }, + reset: { + description: 'reset to zero', + args: {}, + }, + }, + }, + }, + }, + }); + expect(result.methods.makeCounter?.returnType).toStrictEqual({ + kind: 'remotable', + spec: { + description: 'a stateful counter', + methods: { + increment: { + description: 'bump and return', + parameters: [], + returnType: { kind: 'number' }, + }, + reset: { + description: 'reset to zero', + parameters: [], + returnType: { kind: 'void' }, + }, + }, + }, + }); + }); + + it('supports interfaces nested inside object returns', () => { + const result: RemotableSpec = methodsToRemotableSpec({ + methods: { + buy: { + description: 'buy something', + args: {}, + returns: { + type: 'object', + description: 'purchase result', + properties: { + handle: { type: 'string' }, + reviser: { + type: 'interface', + description: 'follow-up reviser', + methods: { + revise: { + description: 'produce next revision', + args: { feedback: { type: 'string' } }, + returns: { type: 'string' }, + }, + }, + }, + }, + required: ['handle', 'reviser'], + }, + }, + }, + }); + const returnType = result.methods.buy?.returnType; + expect(returnType?.kind).toBe('object'); + const objectReturn = returnType as Extract< + typeof returnType, + { kind: 'object' } + >; + expect(objectReturn.spec.properties.reviser?.type).toStrictEqual({ + kind: 'remotable', + spec: { + description: 'follow-up reviser', + methods: { + revise: { + description: 'produce next revision', + parameters: [{ description: 'feedback', type: { kind: 'string' } }], + returnType: { kind: 'string' }, + }, + }, + }, + }); + }); }); diff --git a/packages/service-discovery-types/src/method-schema-convert.ts b/packages/service-discovery-types/src/method-schema-convert.ts index e2900d68d9..efcf2ce24e 100644 --- a/packages/service-discovery-types/src/method-schema-convert.ts +++ b/packages/service-discovery-types/src/method-schema-convert.ts @@ -11,8 +11,10 @@ * use the iteration order of the args record, and we drop the names. The * names are preserved as `ValueSpec.description` if no description was * otherwise present, so they remain human-readable. - * - `JsonSchema` has no notion of `remotable`, `null`, `void`, `bigint`, - * `unknown`, or `union`. The converter never emits those kinds. + * - `JsonSchema` has no notion of `null`, `void`, `bigint`, `unknown`, or + * `union`. The converter never emits those kinds. Interfaces + * (`type: 'interface'`) do have a `JsonSchema` counterpart and translate + * to `RemotableTypeSpec`. */ import type { JsonSchema, MethodSchema } from '@metamask/kernel-utils'; @@ -47,6 +49,11 @@ export function jsonSchemaToTypeSpec(schema: JsonSchema): TypeSpec { kind: 'object', spec: jsonSchemaToObjectSpec(schema), }; + case 'interface': + return { + kind: 'remotable', + spec: interfaceJsonSchemaToRemotableSpec(schema), + }; default: { // Exhaustive: JsonSchema is a closed union. const unreachable: never = schema; @@ -55,6 +62,28 @@ export function jsonSchemaToTypeSpec(schema: JsonSchema): TypeSpec { } } +/** + * Convert an interface-typed `JsonSchema` to a `RemotableSpec`. The + * schema describes an object with methods; each method is converted + * recursively via {@link methodSchemaToMethodSpec}. + * + * @param schema - The source interface-typed JsonSchema. + * @returns The equivalent RemotableSpec. + */ +export function interfaceJsonSchemaToRemotableSpec( + schema: Extract, +): RemotableSpec { + const methods: Record = {}; + for (const [name, methodSchema] of Object.entries(schema.methods)) { + methods[name] = methodSchemaToMethodSpec(methodSchema); + } + const out: RemotableSpec = { methods }; + if (schema.description !== undefined) { + out.description = schema.description; + } + return out; +} + /** * Convert an object-typed `JsonSchema` to an `ObjectSpec`. * From 0b270675cbf388260510fef099208a94624597ef Mon Sep 17 00:00:00 2001 From: Chip Morningstar Date: Tue, 4 Aug 2026 16:15:27 -0700 Subject: [PATCH 07/24] docs: changelog entries for the IOListener and JsonSchema interface work Co-Authored-By: Claude Opus 4.7 --- packages/kernel-node-runtime/CHANGELOG.md | 1 + packages/kernel-utils/CHANGELOG.md | 1 + packages/ocap-kernel/CHANGELOG.md | 4 ++++ packages/service-discovery-types/CHANGELOG.md | 1 + 4 files changed, 7 insertions(+) diff --git a/packages/kernel-node-runtime/CHANGELOG.md b/packages/kernel-node-runtime/CHANGELOG.md index 67ae0c9adc..ed27586136 100644 --- a/packages/kernel-node-runtime/CHANGELOG.md +++ b/packages/kernel-node-runtime/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- **BREAKING:** `makeIOChannelFactory` is now `makeIOListenerFactory`, and `makeSocketIOChannel` is now `makeSocketIOListener`. The Unix-socket server hands each connection to `accept()` as its own `IOChannel`, whose receive buffer, decoder, line queue, and reader queue are local to that connection, so any number of peers can be served concurrently. Connections arriving before `accept()` is called are queued rather than dropped. Gone with the single-client design: the shared `currentSocket`, the session-boundary latch, the merged line queue, and the `socket.destroy()` that rejected every second connection ([#1007](https://github.com/MetaMask/ocap-kernel/pull/1007)) - **BREAKING:** Drop `platformOptions.fetch` from `makeNodeJsVatSupervisor` ([#942](https://github.com/MetaMask/ocap-kernel/pull/942)) - `fetch` is now a vat endowment; stub `globalThis.fetch` directly if needed diff --git a/packages/kernel-utils/CHANGELOG.md b/packages/kernel-utils/CHANGELOG.md index 88f885e05e..c84728c27b 100644 --- a/packages/kernel-utils/CHANGELOG.md +++ b/packages/kernel-utils/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- Add an `interface` variant to `JsonSchema` — `{ type: 'interface', description?, methods }` — describing an object whose methods can be invoked, so a method that returns an object reference can declare that object's API inline and a client need not make a second round-trip to discover it. The `methods` field is recursive, so a returned interface can itself return interfaces. The variant describes an _interface_; whether the reference to the object is unforgeable is a property of the reference plumbing, not of the description ([#1007](https://github.com/MetaMask/ocap-kernel/pull/1007)) - Add a `./described` export with a combinator namespace `S` (`S.string`/`S.number`/`S.boolean`/`S.arrayOf`/`S.record`/`S.object`/`S.nothing` leaves, plus `S.arg`/`S.method`/`S.interface`) that authors an `@endo/patterns` interface guard and a matching `MethodSchema` from a single source, so a discoverable exo's enforced shape and its `__getDescription__` hint cannot drift ([#958](https://github.com/MetaMask/ocap-kernel/pull/958)) - Add an optional `required` field to `MethodSchema` (mirroring `required` on object `JsonSchema`) naming which arguments are required, and a `{ required }` option on `methodArgsToStruct` that validates unlisted arguments as optional, so a method's argument schema can faithfully represent the optional trailing arguments its guard already allows ([#958](https://github.com/MetaMask/ocap-kernel/pull/958)) - Add `getLibp2pRelayHome()` to the `./nodejs` exports, returning the libp2p relay's bookkeeping directory (default `~/.libp2p-relay`, overridable via `$LIBP2P_RELAY_HOME`) — kept separate from `$OCAP_HOME` so one relay can serve daemons with different OCAP_HOMEs ([#952](https://github.com/MetaMask/ocap-kernel/pull/952)) diff --git a/packages/ocap-kernel/CHANGELOG.md b/packages/ocap-kernel/CHANGELOG.md index de6c99c0e5..4cefbaee45 100644 --- a/packages/ocap-kernel/CHANGELOG.md +++ b/packages/ocap-kernel/CHANGELOG.md @@ -9,6 +9,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- Add `IOListener`, an endpoint peers connect to that yields one `IOChannel` per connection via `accept()`, replacing the previous one-client-at-a-time channel. Each accepted connection is a distinct object, so holding one conveys no way to reach another, and `direction` is enforced per connection. `accept()` resolves `null` once the listener is closed so an accept loop can terminate rather than hang ([#1007](https://github.com/MetaMask/ocap-kernel/pull/1007)) +- Add `KernelServiceManager.registerAnonymousKernelObject()` / `releaseAnonymousKernelObject()`, which make a kernel-hosted object routable by kref without entering it in the service-name index, so it has no name in the global service namespace and cannot be requested via a cluster config's `services` list. Used to host accepted IO connections, whose authority comes from holding the reference ([#1007](https://github.com/MetaMask/ocap-kernel/pull/1007)) - Add `fetch`, `Request`, `Headers`, and `Response` to available vat endowments ([#942](https://github.com/MetaMask/ocap-kernel/pull/942)) - Add `VatConfig.network: { allowedHosts: string[] }`; requesting `'fetch'` without it rejects `initVat` - Integrate Snaps attenuated endowment factories into vat globals ([#937](https://github.com/MetaMask/ocap-kernel/pull/937)) @@ -25,6 +27,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- **BREAKING:** `Kernel.make`'s `ioChannelFactory` option is now `ioListenerFactory`, and the exported `IOChannelFactory` type is replaced by `IOListener` and `IOListenerFactory`. A cluster config's `io` entries now create listeners; vats call `accept()` to obtain a channel instead of reading and writing the endowment directly ([#1007](https://github.com/MetaMask/ocap-kernel/pull/1007)) - Attribute a failed subcluster vat launch to the specific vat by kernel id and `ClusterConfig` name (e.g. `Failed to launch vat v3 (bob)`), preserving the original error as the `cause` ([#975](https://github.com/MetaMask/ocap-kernel/pull/975)) - **BREAKING:** Remove `VatConfig.platformConfig.fetch` — migrate to `globals: ['fetch', ...]` + `network.allowedHosts` ([#942](https://github.com/MetaMask/ocap-kernel/pull/942)) - **BREAKING:** `MakeAllowedGlobals` now takes a `{ logger }` options bag ([#942](https://github.com/MetaMask/ocap-kernel/pull/942)) @@ -34,6 +37,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- The kernel run queue no longer strands messages after a restart. `runQueueLengthCache` uses a negative value to mean "unknown, re-read from the database", but `enqueueRun`/`dequeueRun` adjusted it arithmetically without materializing it first — so an enqueue while the cache held its startup value of `-1` produced `0` for a queue that actually held an item, and because `0` is not negative it was never re-read. The run loop then saw an empty queue, went to sleep, and stranded everything queued behind it, with no error and no log. The run loop is also now woken by any non-empty queue rather than only by the empty-to-one transition, so a drifted count cannot silently lose the wakeup ([#1007](https://github.com/MetaMask/ocap-kernel/pull/1007)) - Deserialize CapData rejections in `Kernel.queueMessage` so vat errors surface as plain `Error` objects to all callers ([#928](https://github.com/MetaMask/ocap-kernel/pull/928)) - Detect peer restart across receiver state loss so the receiving kernel no longer silently drops a restarted peer's `seq=1` messages ([#948](https://github.com/MetaMask/ocap-kernel/pull/948)) - Persist the peer's last-observed incarnation and compare it on every successful handshake; on a detected restart, clear the peer's c-list contributions and reject the promises it was deciding before the new incarnation reuses any erefs diff --git a/packages/service-discovery-types/CHANGELOG.md b/packages/service-discovery-types/CHANGELOG.md index e774a6d5af..5dfaa4e353 100644 --- a/packages/service-discovery-types/CHANGELOG.md +++ b/packages/service-discovery-types/CHANGELOG.md @@ -16,6 +16,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- Add `interfaceJsonSchemaToRemotableSpec`, and teach the `JsonSchema` converter to translate the new `interface` variant to a `RemotableSpec`, so `remotable` is no longer among the kinds `JsonSchema` cannot express ([#1007](https://github.com/MetaMask/ocap-kernel/pull/1007)) - `methodSchemaToMethodSpec` marks any parameter absent from the source `MethodSchema.required` as `optional` on its emitted `ValueSpec`, instead of treating every parameter as required ([#958](https://github.com/MetaMask/ocap-kernel/pull/958)) [Unreleased]: https://github.com/MetaMask/ocap-kernel/ From 1db4f5c7866ec9102c030230017b039598d362a4 Mon Sep 17 00:00:00 2001 From: Chip Morningstar Date: Tue, 4 Aug 2026 16:53:05 -0700 Subject: [PATCH 08/24] test(kernel-utils): cover the interface JsonSchema validator MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The interface case validates that the value is a non-null object and nothing more — the declared `methods` describe the object for the caller rather than a shape to enforce here, since whether the object honours them is only discoverable by invoking it. Covers both halves: any object passes regardless of its methods, and every non-object is rejected. Co-Authored-By: Claude Opus 4.7 --- .../src/json-schema-to-struct.test.ts | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/packages/kernel-utils/src/json-schema-to-struct.test.ts b/packages/kernel-utils/src/json-schema-to-struct.test.ts index 8530c21c15..f6be0a50d8 100644 --- a/packages/kernel-utils/src/json-schema-to-struct.test.ts +++ b/packages/kernel-utils/src/json-schema-to-struct.test.ts @@ -51,6 +51,39 @@ describe('jsonSchemaToStruct', () => { }); assert({ a: 1, extra: 'ignored' }, struct); }); + + describe('interface', () => { + const interfaceSchema = { + type: 'interface', + description: 'a reviser', + methods: { + revise: { description: 'revise it', args: {} }, + }, + } as const; + + it('accepts any object reference without introspecting its methods', () => { + const struct = jsonSchemaToStruct(interfaceSchema); + // The declared `methods` are a description for the caller, not a + // shape to enforce here: whether the object honours them is only + // discoverable by invoking it, which is the receiver's business. + assert({}, struct); + assert({ revise: () => undefined }, struct); + assert({ somethingElse: 1 }, struct); + }); + + it.each([ + ['a string', 'not an object'], + ['a number', 42], + ['a boolean', true], + ['null', null], + ['undefined', undefined], + ])('rejects %s', (_label, value) => { + const struct = jsonSchemaToStruct(interfaceSchema); + expect(() => assert(value, struct)).toThrow( + /Expected an object reference/u, + ); + }); + }); }); describe('methodArgsToStruct', () => { From 32ea01dd64e449a6ca599c215853df79e876f80e Mon Sep 17 00:00:00 2001 From: Chip Morningstar Date: Tue, 4 Aug 2026 17:03:31 -0700 Subject: [PATCH 09/24] feat(ocap-jsonrpc-vat): line-delimited JSON-RPC over a Unix socket A general-purpose building block: a vat that serves line-delimited JSON-RPC 2.0 on an IOListener endowment, so local non-vat processes can reach kernel objects without shell-execing the CLI per call. Two methods. `redeemURL(url)` redeems an OCAP URL through the kernel's `ocapURLRedemptionService` and returns a name for the resulting reference. `send(target, method, args)` invokes `E(target)[method](...args)`, expanding names in `args` to live references and substituting any remotable in the result for a name. Names are `@@j` sigil strings scoped to one connection. That scoping is load-bearing rather than incidental: the client is outside the ocap world, so the names it holds are plain forgeable strings, and confining them to a connection is what stops one client naming another's references. A forged name simply misses that client's own table. Each connection therefore gets its own bridge, and the accept loop serves connections concurrently without one client's traffic blocking another's. Co-Authored-By: Claude Opus 4.7 --- packages/ocap-jsonrpc-vat/CHANGELOG.md | 17 + packages/ocap-jsonrpc-vat/README.md | 39 ++ packages/ocap-jsonrpc-vat/package.json | 90 ++++ .../ocap-jsonrpc-vat/scripts/VPS-REHEARSAL.md | 87 ++++ packages/ocap-jsonrpc-vat/scripts/probe.mjs | 142 ++++++ .../scripts/start-ocap-jsonrpc-vat.sh | 165 +++++++ packages/ocap-jsonrpc-vat/src/bridge.test.ts | 422 ++++++++++++++++++ packages/ocap-jsonrpc-vat/src/bridge.ts | 327 ++++++++++++++ .../src/cluster-config.test.ts | 43 ++ .../ocap-jsonrpc-vat/src/cluster-config.ts | 50 +++ packages/ocap-jsonrpc-vat/src/index.ts | 18 + .../ocap-jsonrpc-vat/src/json-rpc.test.ts | 131 ++++++ packages/ocap-jsonrpc-vat/src/json-rpc.ts | 155 +++++++ packages/ocap-jsonrpc-vat/src/vat/index.ts | 272 +++++++++++ packages/ocap-jsonrpc-vat/tsconfig.build.json | 16 + packages/ocap-jsonrpc-vat/tsconfig.json | 19 + packages/ocap-jsonrpc-vat/typedoc.json | 8 + packages/ocap-jsonrpc-vat/vitest.config.ts | 22 + tsconfig.build.json | 1 + tsconfig.json | 1 + yarn.lock | 40 ++ 21 files changed, 2065 insertions(+) create mode 100644 packages/ocap-jsonrpc-vat/CHANGELOG.md create mode 100644 packages/ocap-jsonrpc-vat/README.md create mode 100644 packages/ocap-jsonrpc-vat/package.json create mode 100644 packages/ocap-jsonrpc-vat/scripts/VPS-REHEARSAL.md create mode 100644 packages/ocap-jsonrpc-vat/scripts/probe.mjs create mode 100755 packages/ocap-jsonrpc-vat/scripts/start-ocap-jsonrpc-vat.sh create mode 100644 packages/ocap-jsonrpc-vat/src/bridge.test.ts create mode 100644 packages/ocap-jsonrpc-vat/src/bridge.ts create mode 100644 packages/ocap-jsonrpc-vat/src/cluster-config.test.ts create mode 100644 packages/ocap-jsonrpc-vat/src/cluster-config.ts create mode 100644 packages/ocap-jsonrpc-vat/src/index.ts create mode 100644 packages/ocap-jsonrpc-vat/src/json-rpc.test.ts create mode 100644 packages/ocap-jsonrpc-vat/src/json-rpc.ts create mode 100644 packages/ocap-jsonrpc-vat/src/vat/index.ts create mode 100644 packages/ocap-jsonrpc-vat/tsconfig.build.json create mode 100644 packages/ocap-jsonrpc-vat/tsconfig.json create mode 100644 packages/ocap-jsonrpc-vat/typedoc.json create mode 100644 packages/ocap-jsonrpc-vat/vitest.config.ts diff --git a/packages/ocap-jsonrpc-vat/CHANGELOG.md b/packages/ocap-jsonrpc-vat/CHANGELOG.md new file mode 100644 index 0000000000..ebb4c85a15 --- /dev/null +++ b/packages/ocap-jsonrpc-vat/CHANGELOG.md @@ -0,0 +1,17 @@ +# Changelog + +All notable changes to this project will be documented in this file. + +The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), +and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). + +## [Unreleased] + +### Added + +- Initial release: vat serving a line-delimited JSON-RPC 2.0 protocol on a Unix-domain-socket `IOService` endowment + - `redeemURL(url)` redeems an OCAP URL through the kernel's `ocapURLRedemptionService` and returns a sigil name of the form `@@o` referring to the resulting live reference + - `send(target, method, args)` invokes `E(target)[method](...args)` with `@@o` markers in `args` expanded to their live references and any remotable in the result substituted for its sigil name + - Session state is in-memory only and resets on socket disconnect + +[Unreleased]: https://github.com/MetaMask/ocap-kernel/ diff --git a/packages/ocap-jsonrpc-vat/README.md b/packages/ocap-jsonrpc-vat/README.md new file mode 100644 index 0000000000..b86c29a8d0 --- /dev/null +++ b/packages/ocap-jsonrpc-vat/README.md @@ -0,0 +1,39 @@ +# `@ocap/ocap-jsonrpc-vat` + +Ocap kernel vat that exposes access to kernel objects via a JSON-RPC +interface on a Unix-domain socket. Intended as the routine path for +local, non-vat processes (e.g. LLM tool plugins) to redeem OCAP URLs +and send messages to the resulting objects, replacing ad-hoc use of +the kernel-cli's `queueMessage` RPC. + +## Protocol + +The vat serves a line-delimited JSON-RPC 2.0 interface on the socket. +Two methods: + +- `redeemURL({ url: string }) -> "@@j"` + + Redeems `url` through the kernel's `ocapURLRedemptionService` and + returns a sigil name of the form `"@@j1"`, `"@@j2"`, ... referring + to the resulting live reference. Callable at any time. + +- `send({ target: string, method: string, args?: unknown[] }) -> unknown` + + Invokes `E(target)[method](...args)`. The `target` and any nested + `"@@j"` string in `args` is expanded to its live remotable + before dispatch. The awaited result is walked and every remotable + it contains (previously known or newly encountered) is replaced by + its `"@@j"` name in the response. + +Object identity is preserved: an object the caller has already seen +keeps the same `@@j` name across `redeemURL` and `send` calls. + +## Session lifecycle + +The naming table lives in memory only. On socket disconnect the vat +resets its state and awaits a new client; the new client's names +start at `j1` again. + +Restarting the daemon likewise resets the session — this is the +common case, since restart is typically how the operator triggers a +fresh state. diff --git a/packages/ocap-jsonrpc-vat/package.json b/packages/ocap-jsonrpc-vat/package.json new file mode 100644 index 0000000000..68912a38d7 --- /dev/null +++ b/packages/ocap-jsonrpc-vat/package.json @@ -0,0 +1,90 @@ +{ + "name": "@ocap/ocap-jsonrpc-vat", + "version": "0.0.0", + "private": true, + "description": "Ocap kernel vat that exposes access to kernel objects via a JSON-RPC interface on a Unix-domain socket", + "homepage": "https://github.com/MetaMask/ocap-kernel/tree/main/packages/ocap-jsonrpc-vat#readme", + "bugs": { + "url": "https://github.com/MetaMask/ocap-kernel/issues" + }, + "repository": { + "type": "git", + "url": "https://github.com/MetaMask/ocap-kernel.git" + }, + "type": "module", + "exports": { + ".": { + "import": { + "types": "./dist/index.d.mts", + "default": "./dist/index.mjs" + }, + "require": { + "types": "./dist/index.d.cts", + "default": "./dist/index.cjs" + } + }, + "./package.json": "./package.json" + }, + "files": [ + "dist/" + ], + "scripts": { + "build": "ts-bridge --project tsconfig.build.json --no-references --clean", + "bundle-vat": "node ../kernel-cli/dist/app.mjs bundle ./src/vat/index.ts", + "build:docs": "typedoc", + "changelog:validate": "../../scripts/validate-changelog.sh @ocap/ocap-jsonrpc-vat", + "clean": "rimraf --glob './*.tsbuildinfo' ./.eslintcache ./coverage ./dist ./.turbo ./logs", + "lint": "yarn lint:eslint && yarn lint:misc --check && yarn constraints && yarn lint:dependencies", + "lint:dependencies": "depcheck --quiet", + "lint:eslint": "eslint . --cache", + "lint:fix": "yarn lint:eslint --fix && yarn lint:misc --write && yarn constraints --fix && yarn lint:dependencies", + "lint:misc": "prettier --no-error-on-unmatched-pattern '**/*.json' '**/*.md' '**/*.html' '!**/CHANGELOG.old.md' '**/*.yml' '!.yarnrc.yml' '!merged-packages/**' --ignore-path ../../.gitignore --log-level error", + "publish:preview": "yarn npm publish --tag preview", + "test": "vitest run --config vitest.config.ts", + "test:clean": "yarn test --no-cache --coverage.clean", + "test:dev": "yarn test --mode development", + "test:verbose": "yarn test --reporter verbose", + "test:watch": "vitest --config vitest.config.ts", + "test:dev:quiet": "yarn test:dev --reporter @ocap/repo-tools/vitest-reporters/silent" + }, + "dependencies": { + "@endo/eventual-send": "^1.3.4", + "@endo/pass-style": "^1.6.3", + "@metamask/kernel-utils": "workspace:^", + "@metamask/ocap-kernel": "workspace:^" + }, + "devDependencies": { + "@arethetypeswrong/cli": "^0.17.4", + "@metamask/auto-changelog": "^5.3.0", + "@metamask/eslint-config": "^15.0.0", + "@metamask/eslint-config-nodejs": "^15.0.0", + "@metamask/eslint-config-typescript": "^15.0.0", + "@ocap/repo-tools": "workspace:^", + "@ts-bridge/cli": "^0.6.3", + "@ts-bridge/shims": "^0.1.1", + "@typescript-eslint/eslint-plugin": "^8.29.0", + "@typescript-eslint/parser": "^8.29.0", + "@typescript-eslint/utils": "^8.29.0", + "@vitest/eslint-plugin": "^1.6.14", + "depcheck": "^1.4.7", + "eslint": "^9.23.0", + "eslint-config-prettier": "^10.1.1", + "eslint-import-resolver-typescript": "^4.3.1", + "eslint-plugin-import-x": "^4.10.0", + "eslint-plugin-jsdoc": "^50.6.9", + "eslint-plugin-n": "^17.17.0", + "eslint-plugin-prettier": "^5.2.6", + "eslint-plugin-promise": "^7.2.1", + "prettier": "^3.5.3", + "rimraf": "^6.0.1", + "turbo": "^2.9.1", + "typedoc": "^0.28.1", + "typescript": "~5.8.2", + "typescript-eslint": "^8.29.0", + "vite": "^8.0.6", + "vitest": "^4.1.3" + }, + "engines": { + "node": ">=22" + } +} diff --git a/packages/ocap-jsonrpc-vat/scripts/VPS-REHEARSAL.md b/packages/ocap-jsonrpc-vat/scripts/VPS-REHEARSAL.md new file mode 100644 index 0000000000..9e684eab21 --- /dev/null +++ b/packages/ocap-jsonrpc-vat/scripts/VPS-REHEARSAL.md @@ -0,0 +1,87 @@ +# VPS-side rehearsal notes for the ocap-jsonrpc-vat + +The vat replaces the openclaw plugins' shell-execed `ocap daemon +queueMessage`/`redeem-url` calls with a persistent JSON-RPC 2.0 +connection over a Unix socket. On the VPS it lives in the consumer +daemon (`~/.ocap-consumer`), which had no vats previously. + +Both routine restarts and cold resets are automated: + +- `rehearsal-restart-matcher.sh` — routine pre-rehearsal reset (URL, + registry, and vats stay put). Now includes a step 2b that runs + `start-ocap-jsonrpc-vat.sh --home ~/.ocap-consumer`. +- `reset-everything.sh` — cold reset with fresh URLs. Now includes a + step 7b that launches a fresh vat subcluster in the consumer + daemon. + +So the operator does not have to invoke the vat launcher directly in +normal rehearsal flow. The manual launcher (below) is only for +debugging or ad-hoc use. + +## Prerequisites + +- The chip/orchestration-demo branch is checked out at the same path + as before (openclaw plugins install with `-l` from the workspace, + so the branch update is picked up automatically). +- `yarn workspace @metamask/kernel-cli build` and + `yarn workspace @ocap/ocap-jsonrpc-vat build` have run at least + once since the branch update. + +## Manual launch (for debugging) + +```bash +./packages/ocap-jsonrpc-vat/scripts/start-ocap-jsonrpc-vat.sh \ + --home ~/.ocap-consumer +``` + +Confirm the socket: + +```bash +ls -l ~/.ocap-consumer/ocap-jsonrpc.sock +node ./packages/ocap-jsonrpc-vat/scripts/probe.mjs \ + ~/.ocap-consumer/ocap-jsonrpc.sock ocap:some@peer +``` + +The probe should print a `redeemURL` request whose response is either +a `@@j` marker (on success) or a `[KERNEL:DELIVERY_FAILED]` error +if the URL doesn't resolve or remote comms are down. + +## Openclaw plugin config + +For each of the three plugins in `~/.openclaw/openclaw.json` under +`plugins.entries` (`discovery`, `metamask`, `demo`): + +- **Remove** `ocapCliPath`. The plugin's config schema no longer + accepts it — leaving it in will fail plugin registration. +- **Add or change** `ocapHome` to `~/.ocap-consumer`. All three + plugins point at the same consumer-daemon socket. + +Alternatively set `socketPath` explicitly per plugin. + +Example diff: + +```jsonc +"discovery": { + "config": { +- "ocapCliPath": "/root/…/packages/kernel-cli/dist/app.mjs", ++ "ocapHome": "/root/.ocap-consumer", + "matcherUrl": "ocap:…" + } +} +``` + +Then restart openclaw (rehearsal-restart-matcher.sh does this in +step 3). + +## Sanity check before an LLM turn + +- `discovery_list_tracked` should show the matcher URL pre-redeemed + and its ref shown as `@@j` (was previously a kref). +- A `discovery_find_services` turn against the matcher should behave + as before — matcher on VPS, provider vats on laptop are untouched. + +## What changes on the laptop side + +Nothing structural. The laptop's provider vats and consumer daemon +keep their existing OCAP URLs. The plugins on VPS reach them through +the same libp2p path; only the plugin-to-kernel hop changed. diff --git a/packages/ocap-jsonrpc-vat/scripts/probe.mjs b/packages/ocap-jsonrpc-vat/scripts/probe.mjs new file mode 100644 index 0000000000..0efc53c18e --- /dev/null +++ b/packages/ocap-jsonrpc-vat/scripts/probe.mjs @@ -0,0 +1,142 @@ +// Minimal JSON-RPC probe for the ocap JSON-RPC vat. +// +// Connects to the vat's Unix socket. For each URL supplied on the +// command line, sends a `redeemURL` request; if any URLs were supplied, +// follows with a deliberately-invalid `send` to prove the error path +// also works. Prints every request/response pair to stdout. +// +// Usage: +// node scripts/probe.mjs [SOCKET_PATH] [URL ...] +// +// Defaults SOCKET_PATH to ~/.ocap/ocap-jsonrpc.sock and the URL list to +// empty (which exercises just connection setup). + +import net from 'node:net'; +import os from 'node:os'; +import path from 'node:path'; + +const args = process.argv.slice(2); +const defaultSocket = path.join( + process.env.OCAP_HOME ?? path.join(os.homedir(), '.ocap'), + 'ocap-jsonrpc.sock', +); + +let socketPath = defaultSocket; +let urls = []; +if (args.length > 0) { + if (args[0].startsWith('/') || args[0].startsWith('.')) { + socketPath = args[0]; + urls = args.slice(1); + } else { + urls = args; + } +} + +/** + * Connect a client socket, resolving once connected. + * + * @param {string} target - Filesystem path of the Unix socket. + * @returns {Promise} The connected socket. + */ +function connectSocket(target) { + return new Promise((resolve, reject) => { + const client = net.createConnection(target); + client.once('connect', () => resolve(client)); + client.once('error', reject); + }); +} + +/** + * Send one JSON-RPC request over `socket` and await the next line of + * response. The vat's protocol is strictly request/reply on a single + * stream, so this simple wait-for-one-line loop is safe as long as + * callers issue requests serially. + * + * @param {net.Socket} socket - The connected socket. + * @param {object} request - The JSON-RPC request envelope. + * @returns {Promise} The parsed response envelope. + */ +function callOnce(socket, request) { + return new Promise((resolve, reject) => { + let buffer = ''; + /** + * Detach both listeners so we don't double-fire on the socket. + */ + const detach = () => { + // eslint-disable-next-line no-use-before-define + socket.removeListener('data', onData); + // eslint-disable-next-line no-use-before-define + socket.removeListener('error', onError); + }; + /** + * Buffer incoming bytes and resolve on the first complete line. + * + * @param {Buffer} chunk - Incoming data. + */ + const onData = (chunk) => { + buffer += chunk.toString('utf8'); + const newline = buffer.indexOf('\n'); + if (newline < 0) { + return; + } + const line = buffer.slice(0, newline); + detach(); + try { + resolve(JSON.parse(line)); + } catch { + reject(new Error(`bad response line: ${line}`)); + } + }; + /** + * Propagate socket errors as promise rejection. + * + * @param {Error} cause - Socket error. + */ + const onError = (cause) => { + detach(); + reject(cause); + }; + socket.on('data', onData); + socket.once('error', onError); + socket.write(`${JSON.stringify(request)}\n`); + }); +} + +const socket = await connectSocket(socketPath); +process.stderr.write(`connected to ${socketPath}\n`); + +let firstRef; +let nextId = 1; +for (const url of urls) { + const req = { + jsonrpc: '2.0', + id: nextId, + method: 'redeemURL', + params: { url }, + }; + nextId += 1; + process.stdout.write(`→ ${JSON.stringify(req)}\n`); + const reply = await callOnce(socket, req); + process.stdout.write(`← ${JSON.stringify(reply)}\n`); + if (firstRef === undefined && typeof reply?.result === 'string') { + firstRef = reply.result; + } +} + +if (urls.length > 0) { + const sendRequest = { + jsonrpc: '2.0', + id: nextId, + method: 'send', + params: { + target: firstRef ?? '@@j1', + method: '__nonexistent_method__', + args: [], + }, + }; + process.stdout.write(`→ ${JSON.stringify(sendRequest)}\n`); + const sendReply = await callOnce(socket, sendRequest); + process.stdout.write(`← ${JSON.stringify(sendReply)}\n`); +} + +socket.destroy(); diff --git a/packages/ocap-jsonrpc-vat/scripts/start-ocap-jsonrpc-vat.sh b/packages/ocap-jsonrpc-vat/scripts/start-ocap-jsonrpc-vat.sh new file mode 100755 index 0000000000..192e6b5c7a --- /dev/null +++ b/packages/ocap-jsonrpc-vat/scripts/start-ocap-jsonrpc-vat.sh @@ -0,0 +1,165 @@ +#!/usr/bin/env bash +# Launch the ocap JSON-RPC subcluster in a local ocap daemon. +# +# The vat exposes a line-delimited JSON-RPC 2.0 interface on a +# Unix-domain socket under the daemon's home directory. Its two +# methods — `redeemURL(url)` and `send(target,method,args)` — are +# intended as the routine path by which local, non-vat processes +# reach kernel objects, replacing ad-hoc use of the kernel-cli's +# `queueMessage` RPC. +# +# The target daemon is chosen by (in order): +# 1. --home (explicit override on the CLI) +# 2. $OCAP_HOME (environment variable) +# 3. ~/.ocap (default) +# The vat's socket lives at /ocap-jsonrpc.sock. +# +# Prerequisites: the target daemon must already be running and have +# `ocapURLRedemptionService` available (i.e. remote comms initialised +# if you plan to redeem URLs pointing at other peers). +# +# Usage: +# start-ocap-jsonrpc-vat.sh [--home DIR] [--no-build] [--force-reset] + +set -euo pipefail + +SKIP_BUILD=false +FORCE_RESET=false +OCAP_HOME_ARG="" + +usage() { + cat >&2 </ocap-jsonrpc.sock. + --no-build Skip building/bundling the ocap JSON-RPC vat. + --force-reset Force-reset the subcluster if one already exists. + Without this, an existing subcluster is reused as-is. + --help, -h Show this help. +EOF + exit 1 +} + +while [[ $# -gt 0 ]]; do + case "$1" in + --home) + [[ $# -lt 2 ]] && { echo "Error: --home requires a value" >&2; usage; } + OCAP_HOME_ARG="$2"; shift 2 ;; + --no-build) SKIP_BUILD=true; shift ;; + --force-reset) FORCE_RESET=true; shift ;; + --help|-h) usage ;; + *) echo "Error: unknown argument: $1" >&2; usage ;; + esac +done + +info() { echo "[start-ocap-jsonrpc-vat] $*" >&2; } +fail() { echo "[start-ocap-jsonrpc-vat] ERROR: $*" >&2; exit 1; } + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +PKG_DIR="$(cd "$SCRIPT_DIR/.." && pwd)" +REPO_ROOT="$(cd "$PKG_DIR/../.." && pwd)" +OCAP_BIN="$REPO_ROOT/packages/kernel-cli/dist/app.mjs" +BUNDLE_FILE="$PKG_DIR/src/vat/index.bundle" + +OCAP_HOME_DIR="${OCAP_HOME_ARG:-${OCAP_HOME:-${HOME}/.ocap}}" +SOCKET_PATH="$OCAP_HOME_DIR/ocap-jsonrpc.sock" + +if [[ ! -f "$OCAP_BIN" ]]; then + fail "ocap CLI not found at $OCAP_BIN. Run \`yarn workspace @metamask/kernel-cli build\` first." +fi + +if $SKIP_BUILD; then + info "Skipping build (--no-build)" + [[ -f "$BUNDLE_FILE" ]] || fail "Bundle not found at $BUNDLE_FILE. Remove --no-build or build first." +else + info "Building ocap-jsonrpc-vat package..." + (cd "$REPO_ROOT" && yarn workspace @ocap/ocap-jsonrpc-vat build >&2) + info "Bundling vat..." + (cd "$REPO_ROOT" && yarn workspace @ocap/ocap-jsonrpc-vat bundle-vat >&2) +fi + +# All CLI invocations against this daemon go through this wrapper so +# they use the requested home rather than the CLI default. +daemon_cli() { + (cd "$REPO_ROOT" && node "$OCAP_BIN" --home "$OCAP_HOME_DIR" "$@") +} + +# Fast-fail if the daemon isn't up. +if ! daemon_cli daemon exec getStatus >/dev/null 2>&1; then + fail "daemon at $OCAP_HOME_DIR does not respond to \`daemon exec getStatus\`. Start it first." +fi + +# Look up any existing subcluster: reuse it, unless --force-reset was +# passed — in which case terminate it first so we can launch a fresh +# one (kernel state stays, the vat's baggage and @@ name counter go). +EXISTING_ID=$(daemon_cli daemon exec getStatus | node -e " + const raw = require('fs').readFileSync('/dev/stdin','utf8').trim(); + const data = JSON.parse(raw); + const subclusters = data.subclusters ?? []; + const found = subclusters.find( + (sc) => sc?.config?.bootstrap === 'ocapJsonrpcVat', + ); + if (found) { + process.stdout.write(found.id); + } +") + +if [[ -n "$EXISTING_ID" ]]; then + if [[ "$FORCE_RESET" == "true" ]]; then + info "Terminating existing subcluster $EXISTING_ID before relaunch..." + daemon_cli daemon exec terminateSubcluster "$(node -e \ + "process.stdout.write(JSON.stringify({id: process.argv[1]}))" \ + "$EXISTING_ID")" >/dev/null \ + || fail "terminateSubcluster $EXISTING_ID failed" + # Give the kernel a moment to tear down the IO channel. + sleep 0.3 + else + info "Subcluster already exists ($EXISTING_ID); reusing." + if [[ ! -S "$SOCKET_PATH" ]]; then + fail "Existing subcluster claims to be up but socket $SOCKET_PATH is missing." + fi + info "Home: $OCAP_HOME_DIR" + info "Socket: $SOCKET_PATH" + echo "socket: $SOCKET_PATH" + exit 0 + fi +fi + +CONFIG=$(BUNDLE="file://$BUNDLE_FILE" \ + SOCKET="$SOCKET_PATH" \ + node -e " + const config = { + config: { + bootstrap: 'ocapJsonrpcVat', + services: ['ocapURLRedemptionService'], + io: { + socket: { type: 'socket', path: process.env.SOCKET } + }, + vats: { + ocapJsonrpcVat: { bundleSpec: process.env.BUNDLE } + } + } + }; + process.stdout.write(JSON.stringify(config)); +") + +info "Launching subcluster in $OCAP_HOME_DIR..." +daemon_cli daemon exec launchSubcluster "$CONFIG" >/dev/null + +# Give the vat a moment to open its listener before reporting readiness. +for i in $(seq 1 20); do + if [[ -S "$SOCKET_PATH" ]]; then + break + fi + if [[ "$i" -eq 20 ]]; then + fail "Socket $SOCKET_PATH did not appear after 2s. See daemon logs." + fi + sleep 0.1 +done + +info "Vat ready." +info "Home: $OCAP_HOME_DIR" +info "Socket: $SOCKET_PATH" +echo "socket: $SOCKET_PATH" diff --git a/packages/ocap-jsonrpc-vat/src/bridge.test.ts b/packages/ocap-jsonrpc-vat/src/bridge.test.ts new file mode 100644 index 0000000000..eac2cb78c7 --- /dev/null +++ b/packages/ocap-jsonrpc-vat/src/bridge.test.ts @@ -0,0 +1,422 @@ +import { describe, expect, it, vi } from 'vitest'; + +import { makeBridge } from './bridge.ts'; +import type { BridgeHooks } from './bridge.ts'; +import { JSON_RPC_ERROR } from './json-rpc.ts'; + +type FakeRemotable = { __fakeRemotable__: true; label: string }; + +const makeFake = (label: string): FakeRemotable => ({ + __fakeRemotable__: true, + label, +}); + +const isFakeRemotable = (value: unknown): boolean => + typeof value === 'object' && + value !== null && + (value as { __fakeRemotable__?: unknown }).__fakeRemotable__ === true; + +/** + * Build a bridge with configurable hooks. `redeem` and `invoke` + * default to `vi.fn()` so tests can inspect calls. + * + * @param overrides - Any hooks to replace defaults with. + * @returns The bridge plus references to the hook mocks. + */ +function buildBridge(overrides: Partial = {}): { + bridge: ReturnType; + hooks: { + redeem: ReturnType; + invoke: ReturnType; + }; +} { + const redeem = vi.fn(async (_url: string): Promise => makeFake('x')); + const invoke = vi.fn( + async (_target: unknown, _method: string, _args: unknown[]) => + undefined as unknown, + ); + const hooks: BridgeHooks = { + redeem, + invoke, + isRemotable: isFakeRemotable, + ...overrides, + }; + return { + bridge: makeBridge(hooks), + hooks: { redeem, invoke }, + }; +} + +describe('dispatch: request validation', () => { + it('rejects a non-object request', async () => { + const { bridge } = buildBridge(); + const response = await bridge.dispatch(null); + expect(response).toStrictEqual({ + jsonrpc: '2.0', + id: null, + error: { + code: JSON_RPC_ERROR.INVALID_REQUEST, + message: 'not a well-formed JSON-RPC 2.0 request', + }, + }); + }); + + it('rejects a request without jsonrpc: "2.0"', async () => { + const { bridge } = buildBridge(); + const response = await bridge.dispatch({ + id: 1, + method: 'send', + params: {}, + }); + expect(response).toMatchObject({ + error: { code: JSON_RPC_ERROR.INVALID_REQUEST }, + }); + }); + + it('rejects an unknown method', async () => { + const { bridge } = buildBridge(); + const response = await bridge.dispatch({ + jsonrpc: '2.0', + id: 7, + method: 'destroyEverything', + }); + expect(response).toStrictEqual({ + jsonrpc: '2.0', + id: 7, + error: { + code: JSON_RPC_ERROR.METHOD_NOT_FOUND, + message: 'unknown method "destroyEverything"', + }, + }); + }); +}); + +describe('redeemURL', () => { + it('redeems a URL and returns its marker name', async () => { + const alpha = makeFake('alpha'); + const redeem = vi.fn(async (url: string) => { + expect(url).toBe('ocap:alpha'); + return alpha; + }); + const { bridge } = buildBridge({ redeem }); + const response = await bridge.dispatch({ + jsonrpc: '2.0', + id: 1, + method: 'redeemURL', + params: { url: 'ocap:alpha' }, + }); + expect(response).toStrictEqual({ + jsonrpc: '2.0', + id: 1, + result: '@@j1', + }); + }); + + it('reuses the same name across successive redemptions of the same identity', async () => { + const alpha = makeFake('alpha'); + const { bridge } = buildBridge({ redeem: async () => alpha }); + const first = (await bridge.dispatch({ + jsonrpc: '2.0', + id: 1, + method: 'redeemURL', + params: { url: 'ocap:alpha' }, + })) as { result?: unknown }; + const second = (await bridge.dispatch({ + jsonrpc: '2.0', + id: 2, + method: 'redeemURL', + params: { url: 'ocap:alpha' }, + })) as { result?: unknown }; + expect(first.result).toBe('@@j1'); + expect(second.result).toBe('@@j1'); + }); + + it('assigns distinct names for distinct identities', async () => { + const alpha = makeFake('alpha'); + const beta = makeFake('beta'); + const { bridge } = buildBridge({ + redeem: async (url) => (url === 'ocap:alpha' ? alpha : beta), + }); + const first = (await bridge.dispatch({ + jsonrpc: '2.0', + id: 1, + method: 'redeemURL', + params: { url: 'ocap:alpha' }, + })) as { result?: unknown }; + const second = (await bridge.dispatch({ + jsonrpc: '2.0', + id: 2, + method: 'redeemURL', + params: { url: 'ocap:beta' }, + })) as { result?: unknown }; + expect(first.result).toBe('@@j1'); + expect(second.result).toBe('@@j2'); + }); + + it('rejects a non-string url param', async () => { + const { bridge } = buildBridge(); + const response = await bridge.dispatch({ + jsonrpc: '2.0', + id: 1, + method: 'redeemURL', + params: { url: 42 }, + }); + expect(response).toMatchObject({ + error: { code: JSON_RPC_ERROR.INVALID_PARAMS }, + }); + }); + + it('surfaces a redeem() rejection as an application error', async () => { + const { bridge } = buildBridge({ + redeem: async () => { + throw new Error('remote said no'); + }, + }); + const response = await bridge.dispatch({ + jsonrpc: '2.0', + id: 1, + method: 'redeemURL', + params: { url: 'ocap:x' }, + }); + expect(response).toStrictEqual({ + jsonrpc: '2.0', + id: 1, + error: { + code: JSON_RPC_ERROR.APPLICATION_ERROR, + message: 'remote said no', + }, + }); + }); +}); + +describe('send', () => { + it('rejects an unknown @@ target', async () => { + const { bridge } = buildBridge(); + const response = await bridge.dispatch({ + jsonrpc: '2.0', + id: 1, + method: 'send', + params: { target: '@@j99', method: 'noSuch', args: [] }, + }); + expect(response).toMatchObject({ + error: { code: JSON_RPC_ERROR.INVALID_PARAMS }, + }); + }); + + it('rejects a badly-formed target string', async () => { + const { bridge } = buildBridge(); + const response = await bridge.dispatch({ + jsonrpc: '2.0', + id: 1, + method: 'send', + params: { target: 'not-a-marker', method: 'x', args: [] }, + }); + expect(response).toMatchObject({ + error: { code: JSON_RPC_ERROR.INVALID_PARAMS }, + }); + }); + + it('expands marker args to live references before invoking', async () => { + const alpha = makeFake('alpha'); + const beta = makeFake('beta'); + const invoke = vi.fn( + async (_target: unknown, _method: string, _args: unknown[]) => 42, + ); + const { bridge } = buildBridge({ + redeem: async (url) => (url === 'ocap:alpha' ? alpha : beta), + invoke, + }); + await bridge.dispatch({ + jsonrpc: '2.0', + id: 1, + method: 'redeemURL', + params: { url: 'ocap:alpha' }, + }); + await bridge.dispatch({ + jsonrpc: '2.0', + id: 2, + method: 'redeemURL', + params: { url: 'ocap:beta' }, + }); + await bridge.dispatch({ + jsonrpc: '2.0', + id: 3, + method: 'send', + params: { + target: '@@j1', + method: 'handoff', + args: ['@@j2', { via: '@@j2', tag: 'plain' }], + }, + }); + expect(invoke).toHaveBeenCalledWith(alpha, 'handoff', [ + beta, + { via: beta, tag: 'plain' }, + ]); + }); + + it('substitutes remotables in the result with marker strings', async () => { + const alpha = makeFake('alpha'); + const beta = makeFake('beta'); + const { bridge } = buildBridge({ + redeem: async () => alpha, + invoke: async () => ({ echo: 'ok', partner: beta }), + }); + await bridge.dispatch({ + jsonrpc: '2.0', + id: 1, + method: 'redeemURL', + params: { url: 'ocap:alpha' }, + }); + const response = await bridge.dispatch({ + jsonrpc: '2.0', + id: 2, + method: 'send', + params: { target: '@@j1', method: 'introducePartner', args: [] }, + }); + expect(response).toStrictEqual({ + jsonrpc: '2.0', + id: 2, + result: { echo: 'ok', partner: '@@j2' }, + }); + }); + + it('reuses names for objects seen previously', async () => { + const alpha = makeFake('alpha'); + const beta = makeFake('beta'); + const { bridge } = buildBridge({ + redeem: async () => alpha, + invoke: async () => beta, + }); + await bridge.dispatch({ + jsonrpc: '2.0', + id: 1, + method: 'redeemURL', + params: { url: 'ocap:alpha' }, + }); + const first = (await bridge.dispatch({ + jsonrpc: '2.0', + id: 2, + method: 'send', + params: { target: '@@j1', method: 'getBeta', args: [] }, + })) as { result?: unknown }; + expect(first.result).toBe('@@j2'); + const second = (await bridge.dispatch({ + jsonrpc: '2.0', + id: 3, + method: 'send', + params: { target: '@@j1', method: 'getBeta', args: [] }, + })) as { result?: unknown }; + expect(second.result).toBe('@@j2'); + }); + + it('packages an invoke() rejection as an application error', async () => { + const alpha = makeFake('alpha'); + const { bridge } = buildBridge({ + redeem: async () => alpha, + invoke: async () => { + throw new Error('remote said no'); + }, + }); + await bridge.dispatch({ + jsonrpc: '2.0', + id: 1, + method: 'redeemURL', + params: { url: 'ocap:alpha' }, + }); + const response = await bridge.dispatch({ + jsonrpc: '2.0', + id: 2, + method: 'send', + params: { target: '@@j1', method: 'boom', args: [] }, + }); + expect(response).toStrictEqual({ + jsonrpc: '2.0', + id: 2, + error: { + code: JSON_RPC_ERROR.APPLICATION_ERROR, + message: 'remote said no', + }, + }); + }); +}); + +describe('resetSession', () => { + it('discards names so previously-known targets are no longer known', async () => { + const alpha = makeFake('alpha'); + const { bridge } = buildBridge({ redeem: async () => alpha }); + await bridge.dispatch({ + jsonrpc: '2.0', + id: 1, + method: 'redeemURL', + params: { url: 'ocap:alpha' }, + }); + bridge.resetSession(); + const response = await bridge.dispatch({ + jsonrpc: '2.0', + id: 2, + method: 'send', + params: { target: '@@j1', method: 'ping', args: [] }, + }); + expect(response).toMatchObject({ + error: { + code: JSON_RPC_ERROR.INVALID_PARAMS, + message: + 'params.target "@@j1" is not a known reference on this ' + + 'connection (known here: none)', + }, + }); + }); + + it('names the connection and its known refs when a lookup misses', async () => { + const alpha = makeFake('alpha'); + const { bridge } = buildBridge({ + redeem: async () => alpha, + label: 'connection 7', + }); + await bridge.dispatch({ + jsonrpc: '2.0', + id: 1, + method: 'redeemURL', + params: { url: 'ocap:alpha' }, + }); + const response = await bridge.dispatch({ + jsonrpc: '2.0', + id: 2, + method: 'send', + params: { target: '@@j9', method: 'ping', args: [] }, + }); + // The usual cause is a name minted on a different connection, so the + // message has to say which connection is complaining and what it holds. + expect(response).toMatchObject({ + error: { + code: JSON_RPC_ERROR.INVALID_PARAMS, + message: + 'params.target "@@j9" is not a known reference on ' + + 'connection 7 (known here: @@j1)', + }, + }); + }); + + it('resets the name counter so o1 is reallocated fresh', async () => { + const alpha = makeFake('alpha'); + const gamma = makeFake('gamma'); + const { bridge } = buildBridge({ + redeem: async (url) => (url === 'ocap:alpha' ? alpha : gamma), + }); + const first = (await bridge.dispatch({ + jsonrpc: '2.0', + id: 1, + method: 'redeemURL', + params: { url: 'ocap:alpha' }, + })) as { result?: unknown }; + expect(first.result).toBe('@@j1'); + bridge.resetSession(); + const second = (await bridge.dispatch({ + jsonrpc: '2.0', + id: 2, + method: 'redeemURL', + params: { url: 'ocap:gamma' }, + })) as { result?: unknown }; + expect(second.result).toBe('@@j1'); + }); +}); diff --git a/packages/ocap-jsonrpc-vat/src/bridge.ts b/packages/ocap-jsonrpc-vat/src/bridge.ts new file mode 100644 index 0000000000..82d12c1633 --- /dev/null +++ b/packages/ocap-jsonrpc-vat/src/bridge.ts @@ -0,0 +1,327 @@ +/** + * Bridge core: the state machine and per-request dispatch used by the + * ocap JSON-RPC vat. + * + * Factored out of the vat body so its behavior can be exercised in a plain + * Node test environment. The vat wraps this factory with an IOService-driven + * read/dispatch/write loop; production hooks bind `redeem` and `invoke` to + * `E(...)` calls, tests pass in plain-function stand-ins. + */ + +import type { JsonRpcId, JsonRpcRequest, JsonRpcResponse } from './json-rpc.ts'; +import { + JSON_RPC_ERROR, + MARKER_PREFIX, + BridgeRpcError, + expandMarkers, + substituteRemotables, +} from './json-rpc.ts'; + +export type BridgeHooks = { + /** Redeem an OCAP URL to a live reference. */ + redeem: (url: string) => Promise; + /** + * Send `method` with `args` to `target` and await the resolved result. + * In production this is `E(target)[method](...args)`. + */ + invoke: ( + target: unknown, + method: string, + args: unknown[], + ) => Promise; + /** + * Predicate identifying values the response walker should replace with + * `"@@j"` sigil strings. Production wires this to `passStyleOf`. + */ + isRemotable: (value: unknown) => boolean; + /** + * Optional label identifying which connection this bridge serves, used + * in error messages. + * + * Names are scoped to a connection, so "unknown reference" almost + * always means the name was minted on a *different* connection than + * the one asking. Without the label that has to be reconstructed by + * correlating kernel refs in the daemon log, which is a lot of work to + * learn something the error could simply have said. + */ + label?: string | undefined; +}; + +export type Bridge = { + /** + * Handle one already-parsed JSON-RPC request and return the response. + * Never throws; all errors are packaged as JSON-RPC error responses. + */ + dispatch: (request: unknown) => Promise; + /** + * Discard the naming table. Invoked by the vat when the socket client + * disconnects so the next connection begins with fresh names. + */ + resetSession: () => void; +}; + +/** + * Construct a bridge with an empty naming table. + * + * @param hooks - Callbacks that bridge into the environment (URL + * redemption, message send, remotable identification). + * @returns The bridge control interface. + */ +export function makeBridge(hooks: BridgeHooks): Bridge { + let nameToObj = new Map(); + let objToName = new Map(); + let nextObjId = 0; + + /** + * Describe this bridge for error messages. + * + * @returns The connection label, or a generic phrase when unlabelled. + */ + const where = (): string => hooks.label ?? 'this connection'; + + const resetSession = (): void => { + nameToObj = new Map(); + objToName = new Map(); + nextObjId = 0; + }; + + const assignName = (obj: unknown): string => { + const existing = objToName.get(obj); + if (existing !== undefined) { + return existing; + } + nextObjId += 1; + const name = `j${nextObjId}`; + nameToObj.set(name, obj); + objToName.set(obj, name); + return name; + }; + + const resolveName = (name: string): unknown => nameToObj.get(name); + + const handleRedeemURL = async (params: unknown): Promise => { + const url = requireUrlString(params); + const obj = await hooks.redeem(url); + return `${MARKER_PREFIX}${assignName(obj)}`; + }; + + const handleSend = async (params: unknown): Promise => { + const { target, method, args } = requireSendParams(params); + const targetObj = resolveName(target); + if (targetObj === undefined) { + // Report which connection failed to resolve the name and what it + // does hold. Names are per-connection, so the usual cause is a name + // minted on a different connection than the one now using it. + const known = [...nameToObj.keys()] + .map((name) => `${MARKER_PREFIX}${name}`) + .join(', '); + throw new BridgeRpcError( + JSON_RPC_ERROR.INVALID_PARAMS, + `params.target "@@${target}" is not a known reference on ` + + `${where()} (known here: ${known || 'none'})`, + ); + } + const expandedArgs = expandMarkers(args, resolveName) as unknown[]; + const result = await hooks.invoke(targetObj, method, expandedArgs); + return substituteRemotables(result, hooks.isRemotable, assignName); + }; + + const dispatch = async (request: unknown): Promise => { + const id = extractId(request); + if (!isJsonRpcRequest(request)) { + return errorResponse( + id, + JSON_RPC_ERROR.INVALID_REQUEST, + 'not a well-formed JSON-RPC 2.0 request', + ); + } + try { + switch (request.method) { + case 'redeemURL': + return successResponse( + request.id, + await handleRedeemURL(request.params), + ); + case 'send': + return successResponse(request.id, await handleSend(request.params)); + default: + return errorResponse( + request.id, + JSON_RPC_ERROR.METHOD_NOT_FOUND, + `unknown method "${request.method}"`, + ); + } + } catch (error) { + if (error instanceof BridgeRpcError) { + return errorResponse(request.id, error.code, error.message, error.data); + } + const message = error instanceof Error ? error.message : String(error); + return errorResponse( + request.id, + JSON_RPC_ERROR.APPLICATION_ERROR, + message, + ); + } + }; + + return { dispatch, resetSession }; +} + +/** + * Validate `redeemURL`'s params bag and return the URL string. + * + * @param params - The raw `params` field from the request. + * @returns The validated URL. + */ +function requireUrlString(params: unknown): string { + if ( + typeof params !== 'object' || + params === null || + typeof (params as { url?: unknown }).url !== 'string' + ) { + throw new BridgeRpcError( + JSON_RPC_ERROR.INVALID_PARAMS, + 'params.url must be a string', + ); + } + return (params as { url: string }).url; +} + +/** + * Validate `send`'s params bag and return the extracted call target, + * method name, and args array. + * + * @param params - The raw `params` field from the request. + * @returns The validated send arguments; `target` is the NAME (without + * the `@@` sigil) and `args` defaults to `[]` when omitted. + */ +function requireSendParams(params: unknown): { + target: string; + method: string; + args: unknown[]; +} { + if (typeof params !== 'object' || params === null) { + throw new BridgeRpcError( + JSON_RPC_ERROR.INVALID_PARAMS, + 'params must be an object', + ); + } + const bag = params as { + target?: unknown; + method?: unknown; + args?: unknown; + }; + if (typeof bag.target !== 'string') { + throw new BridgeRpcError( + JSON_RPC_ERROR.INVALID_PARAMS, + 'params.target must be a string', + ); + } + const match = /^@@([A-Za-z0-9]+)$/u.exec(bag.target); + if (!match) { + throw new BridgeRpcError( + JSON_RPC_ERROR.INVALID_PARAMS, + 'params.target must be a marker string like "@@j1"', + ); + } + if (typeof bag.method !== 'string') { + throw new BridgeRpcError( + JSON_RPC_ERROR.INVALID_PARAMS, + 'params.method must be a string', + ); + } + if (bag.args !== undefined && !Array.isArray(bag.args)) { + throw new BridgeRpcError( + JSON_RPC_ERROR.INVALID_PARAMS, + 'params.args must be an array', + ); + } + return { + target: match[1] as string, + method: bag.method, + args: (bag.args as unknown[] | undefined) ?? [], + }; +} + +/** + * Type-guard for a well-formed JSON-RPC 2.0 request envelope. + * + * @param value - Candidate parsed-JSON value. + * @returns True iff `value` has the required envelope fields. + */ +function isJsonRpcRequest(value: unknown): value is JsonRpcRequest { + if (typeof value !== 'object' || value === null) { + return false; + } + const bag = value as { + jsonrpc?: unknown; + method?: unknown; + id?: unknown; + }; + if (bag.jsonrpc !== '2.0' || typeof bag.method !== 'string') { + return false; + } + if ( + bag.id !== null && + typeof bag.id !== 'number' && + typeof bag.id !== 'string' && + bag.id !== undefined + ) { + return false; + } + return true; +} + +/** + * Best-effort extraction of the request `id`, used when the request + * fails validation and must be echoed on the error response. + * + * @param value - Candidate parsed-JSON value. + * @returns The id, or `null` if none is recoverable. + */ +function extractId(value: unknown): JsonRpcId { + if (typeof value !== 'object' || value === null) { + return null; + } + const { id } = value as { id?: unknown }; + if (id === null || typeof id === 'number' || typeof id === 'string') { + return id; + } + return null; +} + +/** + * Build a JSON-RPC success response. + * + * @param id - The request id to echo. + * @param result - The result payload. + * @returns The response envelope. + */ +function successResponse(id: JsonRpcId, result: unknown): JsonRpcResponse { + return { jsonrpc: '2.0', id, result }; +} + +/** + * Build a JSON-RPC error response. + * + * @param id - The request id to echo. + * @param code - JSON-RPC error code (see `JSON_RPC_ERROR`). + * @param message - Human-readable error description. + * @param data - Optional additional error data. + * @returns The response envelope. + */ +function errorResponse( + id: JsonRpcId, + code: number, + message: string, + data?: unknown, +): JsonRpcResponse { + const error: { code: number; message: string; data?: unknown } = { + code, + message, + }; + if (data !== undefined) { + error.data = data; + } + return { jsonrpc: '2.0', id, error }; +} diff --git a/packages/ocap-jsonrpc-vat/src/cluster-config.test.ts b/packages/ocap-jsonrpc-vat/src/cluster-config.test.ts new file mode 100644 index 0000000000..aa201ed936 --- /dev/null +++ b/packages/ocap-jsonrpc-vat/src/cluster-config.test.ts @@ -0,0 +1,43 @@ +import { describe, expect, it } from 'vitest'; + +import { + OCAP_JSONRPC_BUNDLE_FILENAME, + OCAP_JSONRPC_SOCKET_CHANNEL, + OCAP_JSONRPC_VAT_NAME, + makeOcapJsonrpcClusterConfig, +} from './cluster-config.ts'; + +describe('makeOcapJsonrpcClusterConfig', () => { + it('produces a config with the ocap JSON-RPC vat as the bootstrap', () => { + const config = makeOcapJsonrpcClusterConfig({ + bundleBaseUrl: 'file:///tmp/jsonrpc', + socketPath: '/tmp/ocap-jsonrpc.sock', + }); + expect(config.bootstrap).toBe(OCAP_JSONRPC_VAT_NAME); + expect(config.services).toStrictEqual(['ocapURLRedemptionService']); + expect(config.io?.[OCAP_JSONRPC_SOCKET_CHANNEL]).toStrictEqual({ + type: 'socket', + path: '/tmp/ocap-jsonrpc.sock', + }); + expect(config.vats[OCAP_JSONRPC_VAT_NAME]?.bundleSpec).toBe( + `file:///tmp/jsonrpc/${OCAP_JSONRPC_BUNDLE_FILENAME}`, + ); + }); + + it('defaults forceReset to false', () => { + const config = makeOcapJsonrpcClusterConfig({ + bundleBaseUrl: 'x', + socketPath: '/x.sock', + }); + expect(config.forceReset).toBe(false); + }); + + it('passes forceReset through when set', () => { + const config = makeOcapJsonrpcClusterConfig({ + bundleBaseUrl: 'x', + socketPath: '/x.sock', + forceReset: true, + }); + expect(config.forceReset).toBe(true); + }); +}); diff --git a/packages/ocap-jsonrpc-vat/src/cluster-config.ts b/packages/ocap-jsonrpc-vat/src/cluster-config.ts new file mode 100644 index 0000000000..80bf01f9ce --- /dev/null +++ b/packages/ocap-jsonrpc-vat/src/cluster-config.ts @@ -0,0 +1,50 @@ +import type { ClusterConfig } from '@metamask/ocap-kernel'; + +/** Vat name for the ocap JSON-RPC vat inside its subcluster. */ +export const OCAP_JSONRPC_VAT_NAME = 'ocapJsonrpcVat'; + +/** + * Filename of the vat bundle produced by `yarn bundle-vat` in this + * package. A launcher supplies a `bundleBaseUrl` pointing at the + * directory containing this file. + */ +export const OCAP_JSONRPC_BUNDLE_FILENAME = 'index.bundle'; + +/** IO channel name the vat expects in its endowments. */ +export const OCAP_JSONRPC_SOCKET_CHANNEL = 'socket'; + +/** + * Build a `ClusterConfig` for the ocap JSON-RPC subcluster. + * + * @param options - Configuration options. + * @param options.bundleBaseUrl - Base URL (or filesystem path) where the + * vat bundle is reachable. The bundle filename is appended. + * @param options.socketPath - Filesystem path for the Unix-domain-socket + * IO channel the vat listens on. + * @param options.forceReset - Whether to reset the subcluster on launch. + * Defaults to `false`. + * @returns A ClusterConfig ready for `kernel.launchSubcluster(...)`. + */ +export function makeOcapJsonrpcClusterConfig(options: { + bundleBaseUrl: string; + socketPath: string; + forceReset?: boolean; +}): ClusterConfig { + const { bundleBaseUrl, socketPath, forceReset = false } = options; + return { + bootstrap: OCAP_JSONRPC_VAT_NAME, + forceReset, + services: ['ocapURLRedemptionService'], + io: { + [OCAP_JSONRPC_SOCKET_CHANNEL]: { + type: 'socket', + path: socketPath, + }, + }, + vats: { + [OCAP_JSONRPC_VAT_NAME]: { + bundleSpec: `${bundleBaseUrl}/${OCAP_JSONRPC_BUNDLE_FILENAME}`, + }, + }, + }; +} diff --git a/packages/ocap-jsonrpc-vat/src/index.ts b/packages/ocap-jsonrpc-vat/src/index.ts new file mode 100644 index 0000000000..0998eb9e86 --- /dev/null +++ b/packages/ocap-jsonrpc-vat/src/index.ts @@ -0,0 +1,18 @@ +export { + OCAP_JSONRPC_BUNDLE_FILENAME, + OCAP_JSONRPC_SOCKET_CHANNEL, + OCAP_JSONRPC_VAT_NAME, + makeOcapJsonrpcClusterConfig, +} from './cluster-config.ts'; + +export { + MARKER_PATTERN, + MARKER_PREFIX, + JSON_RPC_ERROR, + BridgeRpcError, + type JsonRpcId, + type JsonRpcRequest, + type JsonRpcResponse, + type JsonRpcSuccessResponse, + type JsonRpcErrorResponse, +} from './json-rpc.ts'; diff --git a/packages/ocap-jsonrpc-vat/src/json-rpc.test.ts b/packages/ocap-jsonrpc-vat/src/json-rpc.test.ts new file mode 100644 index 0000000000..06f2144b64 --- /dev/null +++ b/packages/ocap-jsonrpc-vat/src/json-rpc.test.ts @@ -0,0 +1,131 @@ +import { describe, expect, it } from 'vitest'; + +import { + MARKER_PREFIX, + BridgeRpcError, + expandMarkers, + substituteRemotables, +} from './json-rpc.ts'; + +/** A stand-in for a remotable, identified by an isRemotable predicate. */ +type FakeRemotable = { __fakeRemotable__: true; id: string }; + +const isFakeRemotable = (value: unknown): boolean => + typeof value === 'object' && + value !== null && + (value as { __fakeRemotable__?: unknown }).__fakeRemotable__ === true; + +const makeFake = (id: string): FakeRemotable => ({ + __fakeRemotable__: true, + id, +}); + +describe('expandMarkers', () => { + const table = new Map([ + ['j1', makeFake('one')], + ['j2', makeFake('two')], + ]); + const resolve = (name: string): unknown => table.get(name); + + it('replaces a top-level marker string', () => { + expect(expandMarkers('@@j1', resolve)).toBe(table.get('j1')); + }); + + it('leaves non-marker strings alone', () => { + expect(expandMarkers('plain string', resolve)).toBe('plain string'); + expect(expandMarkers('@@', resolve)).toBe('@@'); + expect(expandMarkers('prefix@@j1', resolve)).toBe('prefix@@j1'); + expect(expandMarkers('@@j-1', resolve)).toBe('@@j-1'); + }); + + it('walks nested arrays', () => { + const result = expandMarkers(['@@j1', 42, '@@j2'], resolve); + expect(result).toStrictEqual([table.get('j1'), 42, table.get('j2')]); + }); + + it('walks nested objects', () => { + const result = expandMarkers( + { target: '@@j1', label: 'ship', payload: { via: '@@j2' } }, + resolve, + ); + expect(result).toStrictEqual({ + target: table.get('j1'), + label: 'ship', + payload: { via: table.get('j2') }, + }); + }); + + it('passes through primitives untouched', () => { + expect(expandMarkers(42, resolve)).toBe(42); + expect(expandMarkers(null, resolve)).toBeNull(); + expect(expandMarkers(true, resolve)).toBe(true); + }); + + it('throws on an unknown marker', () => { + expect(() => expandMarkers('@@missing', resolve)).toThrow(BridgeRpcError); + expect(() => expandMarkers(['@@missing'], resolve)).toThrow(/@@missing/u); + }); +}); + +describe('substituteRemotables', () => { + it('replaces a top-level remotable with a marker string', () => { + const obj = makeFake('alpha'); + const nameOf = (): string => 'j5'; + expect(substituteRemotables(obj, isFakeRemotable, nameOf)).toBe( + `${MARKER_PREFIX}j5`, + ); + }); + + it('walks nested arrays and objects', () => { + const alpha = makeFake('alpha'); + const beta = makeFake('beta'); + const counter = { n: 0 }; + const assigned = new Map(); + const assign = (obj: unknown): string => { + const existing = assigned.get(obj); + if (existing !== undefined) { + return existing; + } + counter.n += 1; + const name = `j${counter.n}`; + assigned.set(obj, name); + return name; + }; + const result = substituteRemotables( + { via: alpha, args: [beta, 'plain', { echo: alpha }] }, + isFakeRemotable, + assign, + ); + expect(result).toStrictEqual({ + via: '@@j1', + args: ['@@j2', 'plain', { echo: '@@j1' }], + }); + }); + + it('leaves primitives and non-remotable objects alone', () => { + const assign = (): string => 'unused'; + expect(substituteRemotables(42, isFakeRemotable, assign)).toBe(42); + expect(substituteRemotables(null, isFakeRemotable, assign)).toBeNull(); + expect(substituteRemotables('text', isFakeRemotable, assign)).toBe('text'); + expect( + substituteRemotables({ a: 1, b: [2, 3] }, isFakeRemotable, assign), + ).toStrictEqual({ a: 1, b: [2, 3] }); + }); + + it('emits a JSON-serializable tree', () => { + const alpha = makeFake('alpha'); + const assign = (): string => 'j1'; + const tree = substituteRemotables( + { via: alpha, args: [alpha, 'plain'] }, + isFakeRemotable, + assign, + ); + // The whole point of substituteRemotables is that the result can be + // JSON-stringified without special handling. + expect(() => JSON.stringify(tree)).not.toThrow(); + expect(JSON.parse(JSON.stringify(tree))).toStrictEqual({ + via: '@@j1', + args: ['@@j1', 'plain'], + }); + }); +}); diff --git a/packages/ocap-jsonrpc-vat/src/json-rpc.ts b/packages/ocap-jsonrpc-vat/src/json-rpc.ts new file mode 100644 index 0000000000..dc3ea4631b --- /dev/null +++ b/packages/ocap-jsonrpc-vat/src/json-rpc.ts @@ -0,0 +1,155 @@ +/** + * Wire-shape types and walker helpers for the ocap JSON-RPC vat's + * line-delimited JSON-RPC 2.0 protocol. + * + * Object references are named via the sigil convention `"@@NAME"` (NAME + * one or more alphanumeric characters). The mediator assigns names of the + * form `o`; other allocation schemes remain compatible with the walker. + */ + +/** Full sigil string prefix (two `@`). */ +export const MARKER_PREFIX = '@@'; + +/** + * Match a whole string that consists solely of the sigil plus an + * alphanumeric name. Anchored deliberately: an embedded `@@x` is + * plain data. + */ +export const MARKER_PATTERN = /^@@([A-Za-z0-9]+)$/u; + +export type JsonRpcId = number | string | null; + +export type JsonRpcRequest = { + jsonrpc: '2.0'; + id: JsonRpcId; + method: string; + params?: unknown; +}; + +export type JsonRpcSuccessResponse = { + jsonrpc: '2.0'; + id: JsonRpcId; + result: unknown; +}; + +export type JsonRpcErrorResponse = { + jsonrpc: '2.0'; + id: JsonRpcId; + error: { code: number; message: string; data?: unknown }; +}; + +export type JsonRpcResponse = JsonRpcSuccessResponse | JsonRpcErrorResponse; + +/** + * Standard JSON-RPC 2.0 error codes plus a mediator-specific application + * code in the reserved `-32000..-32099` range. + */ +export const JSON_RPC_ERROR = { + PARSE_ERROR: -32700, + INVALID_REQUEST: -32600, + METHOD_NOT_FOUND: -32601, + INVALID_PARAMS: -32602, + INTERNAL_ERROR: -32603, + APPLICATION_ERROR: -32000, +} as const; + +/** + * Thrown from inside the mediator's request handlers to signal the + * intended JSON-RPC error code and message. + */ +export class BridgeRpcError extends Error { + readonly code: number; + + readonly data?: unknown; + + /** + * @param code - JSON-RPC error code to report (see {@link JSON_RPC_ERROR}). + * @param message - Human-readable error description. + * @param data - Optional additional error data to attach. + */ + constructor(code: number, message: string, data?: unknown) { + super(message); + this.code = code; + this.data = data; + } +} + +/** + * Walk `value`, replacing every `"@@NAME"` marker string with + * `resolve(name)`. Descends into plain arrays and record-like objects. + * + * @param value - The value to walk. + * @param resolve - Callback that turns a NAME into a live reference. + * If it returns `undefined` the walker throws — an unknown marker is + * always an error, since silently passing the string through would let + * callers accidentally send the literal `"@@..."` to a service. + * @returns A tree in which markers have been replaced by their live + * references and everything else is unchanged. + */ +export function expandMarkers( + value: unknown, + resolve: (name: string) => unknown, +): unknown { + if (typeof value === 'string') { + const match = MARKER_PATTERN.exec(value); + if (!match) { + return value; + } + const name = match[1] as string; + const resolved = resolve(name); + if (resolved === undefined) { + throw new BridgeRpcError( + JSON_RPC_ERROR.INVALID_PARAMS, + `unknown reference marker "@@${name}"`, + ); + } + return resolved; + } + if (Array.isArray(value)) { + return value.map((item) => expandMarkers(item, resolve)); + } + if (typeof value === 'object' && value !== null) { + const out: Record = {}; + for (const [key, val] of Object.entries(value as Record)) { + out[key] = expandMarkers(val, resolve); + } + return out; + } + return value; +} + +/** + * Walk `value`, replacing every remotable (as identified by + * `isRemotable`) with `"${MARKER_PREFIX}${assign(remotable)}"`. + * Descends into arrays and record-like objects. Primitives pass + * through unchanged. + * + * The result is a JSON-safe tree ready for `JSON.stringify`. + * + * @param value - The value to walk. + * @param isRemotable - Predicate identifying a value that should be + * substituted for a marker. + * @param assign - Callback that turns a remotable into a marker NAME + * (assigning one on first sight, reusing on subsequent sight). + * @returns A JSON-safe tree with remotables replaced by marker strings. + */ +export function substituteRemotables( + value: unknown, + isRemotable: (candidate: unknown) => boolean, + assign: (remotable: unknown) => string, +): unknown { + if (isRemotable(value)) { + return `${MARKER_PREFIX}${assign(value)}`; + } + if (Array.isArray(value)) { + return value.map((item) => substituteRemotables(item, isRemotable, assign)); + } + if (typeof value === 'object' && value !== null) { + const out: Record = {}; + for (const [key, val] of Object.entries(value as Record)) { + out[key] = substituteRemotables(val, isRemotable, assign); + } + return out; + } + return value; +} diff --git a/packages/ocap-jsonrpc-vat/src/vat/index.ts b/packages/ocap-jsonrpc-vat/src/vat/index.ts new file mode 100644 index 0000000000..8ec7e9d9e2 --- /dev/null +++ b/packages/ocap-jsonrpc-vat/src/vat/index.ts @@ -0,0 +1,272 @@ +/** + * Ocap JSON-RPC vat. + * + * Serves a line-delimited JSON-RPC 2.0 interface on a Unix-domain-socket + * `IOListener` endowment named `socket`. External processes connect and + * call `redeemURL(url)` and `send(target, method, args)` — see the + * package README for the wire protocol. + * + * Each connection is served independently, with its own bridge and + * therefore its own `@@j` name table. Two clients can be connected at + * once without either being able to name the other's references: the + * names are closure state of one connection's serve loop, so a forged + * name simply misses that client's own table. Since those names cross a + * non-ocap boundary as plain forgeable strings, per-connection scoping is + * what keeps them from conveying authority they were never granted. + * + * The vat's authority is exactly: + * - the `ocapURLRedemptionService` endowment (for `redeemURL`), + * - whatever references the URLs happen to redeem to, + * - and whatever those references introduce as return values. + * + * The vat has no other public facet: the socket is the sole interface. + */ + +import { E } from '@endo/eventual-send'; +import { passStyleOf } from '@endo/pass-style'; +import { makeDefaultExo } from '@metamask/kernel-utils/exo'; +import type { Baggage, OcapURLRedemptionService } from '@metamask/ocap-kernel'; + +import { makeBridge } from '../bridge.ts'; +import { BridgeRpcError, JSON_RPC_ERROR } from '../json-rpc.ts'; +import type { JsonRpcResponse } from '../json-rpc.ts'; + +/** + * The vat-facing shape of one accepted connection. The kernel-side + * implementation lives in `packages/ocap-kernel/src/io/io-service.ts`. + */ +type IOConnection = { + read: () => Promise; + write: (data: string) => Promise; + close: () => Promise; +}; + +/** + * The vat-facing shape of an `IOListener`. `accept()` resolves to the next + * peer's connection, or `null` once the listener has been closed. Wired + * via the cluster config's `io` block. + */ +type IOListener = { + accept: () => Promise; +}; + +type Services = { + ocapURLRedemptionService: OcapURLRedemptionService; + socket: IOListener; +}; + +/** + * Build the vat's root object. + * + * The `@@j` name table lives in ordinary closure state and is + * intentionally non-durable — each re-incarnation begins with an + * empty table. The services endowments delivered to `bootstrap` are + * stashed in baggage so that on re-incarnation `buildRootObject` can + * restart the socket serve loop without bootstrap having to run + * again (bootstrap only runs once per subcluster lifetime, not on + * every daemon restart). + * + * @param _vatPowers - Unused. + * @param _parameters - Unused. + * @param baggage - Vat baggage. Used to persist the services endowment + * bag so the serve loop can be resumed on re-incarnation. + * @returns The vat root exo. + */ +export function buildRootObject( + _vatPowers: unknown, + _parameters: unknown, + baggage: Baggage, +): unknown { + const log = (...args: unknown[]): void => { + // eslint-disable-next-line no-console + console.log('[ocap-jsonrpc-vat]', ...args); + }; + + const isRemotable = (value: unknown): boolean => { + if (typeof value !== 'object' || value === null) { + return false; + } + try { + const style: string = passStyleOf(value as never); + return style === 'remotable'; + } catch { + return false; + } + }; + + /** + * Read one request line, dispatch it, and write the response. Never + * throws to its caller — decoding, dispatch, and encoding errors are + * either logged and swallowed (when we can't recover an id to reply + * on) or packaged as JSON-RPC error responses. + * + * @param connection - The connection to serve. + * @param dispatch - The bridge's dispatch function. + * @returns 'ok' after processing a request, and 'closed' once the peer + * has gone away or the connection failed. + */ + async function processOne( + connection: IOConnection, + dispatch: (request: unknown) => Promise, + ): Promise<'ok' | 'closed'> { + let line: string | null; + try { + line = await E(connection).read(); + } catch (error) { + log('connection read failed:', error); + return 'closed'; + } + if (line === null) { + return 'closed'; + } + let request: unknown; + try { + request = JSON.parse(line); + } catch (error) { + log('failed to parse request line as JSON; dropping:', error); + return 'ok'; + } + const response = await dispatch(request); + try { + await E(connection).write(JSON.stringify(response)); + } catch (error) { + log('failed to write response:', error); + return 'closed'; + } + return 'ok'; + } + + /** + * Serve one connection for its whole lifetime, with a bridge — and so a + * name table — belonging to it alone. Returns when the peer goes away. + * + * @param services - The endowments delivered by bootstrap. + * @param connection - The connection to serve. + * @param label - Diagnostic label identifying this connection in logs. + */ + async function serveConnection( + services: Services, + connection: IOConnection, + label: string, + ): Promise { + const bridge = makeBridge({ + redeem: async (url) => E(services.ocapURLRedemptionService).redeem(url), + invoke: async (target, method, args) => + // eslint-disable-next-line @typescript-eslint/no-explicit-any + E(target as any)[method](...args), + isRemotable, + label, + }); + try { + for (;;) { + const outcome = await processOne(connection, bridge.dispatch); + if (outcome === 'closed') { + log(`${label}: peer disconnected`); + return; + } + } + } finally { + // Discard this connection's names and let the kernel stop hosting + // it. Nothing else referenced them, so the table dies with the + // connection rather than leaking into whoever connects next. + bridge.resetSession(); + try { + await E(connection).close(); + } catch (error) { + log(`${label}: error closing connection:`, error); + } + } + } + + /** + * Accept connections forever, serving each one concurrently. A peer + * that stalls or floods only affects its own serve loop. + * + * @param services - The endowments delivered by bootstrap. + */ + async function acceptLoop(services: Services): Promise { + let acceptedCount = 0; + for (;;) { + let connection: IOConnection | null; + try { + connection = await E(services.socket).accept(); + } catch (error) { + log('accept failed; ending accept loop:', error); + return; + } + if (!connection) { + log('listener closed; ending accept loop'); + return; + } + acceptedCount += 1; + const label = `connection ${acceptedCount}`; + log(`${label}: accepted`); + // Deliberately not awaited: serving must not block accepting, or a + // single long-lived client would keep everyone else out — which is + // the failure the listener split exists to prevent. + serveConnection(services, connection, label).catch((error) => + log(`${label}: serve loop crashed:`, error), + ); + } + } + + /** + * Kick off the accept loop as a background task. Any crash inside it is + * logged; the vat itself remains alive so it can be introspected. + * + * @param services - The endowments to serve against. + */ + const startAcceptLoop = (services: Services): void => { + acceptLoop(services).catch((error) => log('accept loop crashed:', error)); + }; + + // On re-incarnation (e.g. after `daemon stop`/`daemon start`), + // bootstrap is not re-run — but this `buildRootObject` is. Read the + // previously-stashed services out of baggage and resume accepting. + // + // Only the listener reference has to survive, and it does: the kernel + // re-creates the listener under the same service kref before the vats + // are re-incarnated, so the baggage-held Presence is live again. The + // connections from the previous incarnation are gone, which is correct + // — a socket does not outlive the process on the other end of it. + // + // Deferred to a microtask so vat init completes and the vat is fully + // connected to kernel dispatch before we start issuing E() calls. + if (baggage.has('services')) { + const restored = baggage.get('services') as Services; + Promise.resolve() + .then(() => { + startAcceptLoop(restored); + log('vat re-incarnated; accept loop resumed'); + return undefined; + }) + .catch((error) => + log('failed to resume accept loop on re-incarnation:', error), + ); + } + + return makeDefaultExo('ocapJsonrpcVatRoot', { + async bootstrap(_vats: Record, incoming: Services) { + if (!incoming?.ocapURLRedemptionService) { + throw new BridgeRpcError( + JSON_RPC_ERROR.INTERNAL_ERROR, + 'ocapURLRedemptionService is required', + ); + } + if (!incoming.socket) { + throw new BridgeRpcError( + JSON_RPC_ERROR.INTERNAL_ERROR, + 'socket IOListener is required (configure it in the cluster config under `io.socket`)', + ); + } + if (baggage.has('services')) { + baggage.set('services', incoming); + } else { + baggage.init('services', incoming); + } + startAcceptLoop(incoming); + log('vat bootstrap complete'); + return harden({}); + }, + }); +} diff --git a/packages/ocap-jsonrpc-vat/tsconfig.build.json b/packages/ocap-jsonrpc-vat/tsconfig.build.json new file mode 100644 index 0000000000..6d2c3bc0de --- /dev/null +++ b/packages/ocap-jsonrpc-vat/tsconfig.build.json @@ -0,0 +1,16 @@ +{ + "extends": "../../tsconfig.packages.build.json", + "compilerOptions": { + "baseUrl": "./", + "lib": ["ES2022"], + "outDir": "./dist", + "rootDir": "./src", + "types": [] + }, + "references": [ + { "path": "../kernel-utils/tsconfig.build.json" }, + { "path": "../ocap-kernel/tsconfig.build.json" } + ], + "files": [], + "include": ["./src"] +} diff --git a/packages/ocap-jsonrpc-vat/tsconfig.json b/packages/ocap-jsonrpc-vat/tsconfig.json new file mode 100644 index 0000000000..7d727eadfe --- /dev/null +++ b/packages/ocap-jsonrpc-vat/tsconfig.json @@ -0,0 +1,19 @@ +{ + "extends": "../../tsconfig.packages.json", + "compilerOptions": { + "baseUrl": "./", + "lib": ["ES2022"], + "types": ["vitest"] + }, + "references": [ + { "path": "../kernel-utils" }, + { "path": "../ocap-kernel" }, + { "path": "../repo-tools" } + ], + "include": [ + "../../vitest.config.ts", + "./src", + "./vite.config.ts", + "./vitest.config.ts" + ] +} diff --git a/packages/ocap-jsonrpc-vat/typedoc.json b/packages/ocap-jsonrpc-vat/typedoc.json new file mode 100644 index 0000000000..f8eb78ae1a --- /dev/null +++ b/packages/ocap-jsonrpc-vat/typedoc.json @@ -0,0 +1,8 @@ +{ + "entryPoints": [], + "excludePrivate": true, + "hideGenerator": true, + "out": "docs", + "tsconfig": "./tsconfig.build.json", + "projectDocuments": ["documents/*.md"] +} diff --git a/packages/ocap-jsonrpc-vat/vitest.config.ts b/packages/ocap-jsonrpc-vat/vitest.config.ts new file mode 100644 index 0000000000..7606c3d4d4 --- /dev/null +++ b/packages/ocap-jsonrpc-vat/vitest.config.ts @@ -0,0 +1,22 @@ +import { mergeConfig } from '@ocap/repo-tools/vitest-config'; +import { fileURLToPath } from 'node:url'; +import { defineConfig, defineProject } from 'vitest/config'; + +import defaultConfig from '../../vitest.config.ts'; + +export default defineConfig((args) => { + return mergeConfig( + args, + defaultConfig, + defineProject({ + test: { + name: 'llm-mediator-vat', + setupFiles: [ + fileURLToPath( + import.meta.resolve('@ocap/repo-tools/test-utils/mock-endoify'), + ), + ], + }, + }), + ); +}); diff --git a/tsconfig.build.json b/tsconfig.build.json index 3eacbee1fd..d278b91d30 100644 --- a/tsconfig.build.json +++ b/tsconfig.build.json @@ -17,6 +17,7 @@ { "path": "./packages/llm-bridge/tsconfig.build.json" }, { "path": "./packages/logger/tsconfig.build.json" }, { "path": "./packages/nodejs-test-workers/tsconfig.build.json" }, + { "path": "./packages/ocap-jsonrpc-vat/tsconfig.build.json" }, { "path": "./packages/ocap-kernel/tsconfig.build.json" }, { "path": "./packages/omnium-gatherum/tsconfig.build.json" }, { "path": "./packages/remote-iterables/tsconfig.build.json" }, diff --git a/tsconfig.json b/tsconfig.json index fca57f6805..aeda0aadc2 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -32,6 +32,7 @@ { "path": "./packages/llm-bridge" }, { "path": "./packages/logger" }, { "path": "./packages/nodejs-test-workers" }, + { "path": "./packages/ocap-jsonrpc-vat" }, { "path": "./packages/ocap-kernel" }, { "path": "./packages/omnium-gatherum" }, { "path": "./packages/remote-iterables" }, diff --git a/yarn.lock b/yarn.lock index cdce88f2c3..436bb1784a 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4297,6 +4297,46 @@ __metadata: languageName: unknown linkType: soft +"@ocap/ocap-jsonrpc-vat@workspace:packages/ocap-jsonrpc-vat": + version: 0.0.0-use.local + resolution: "@ocap/ocap-jsonrpc-vat@workspace:packages/ocap-jsonrpc-vat" + dependencies: + "@arethetypeswrong/cli": "npm:^0.17.4" + "@endo/eventual-send": "npm:^1.3.4" + "@endo/pass-style": "npm:^1.6.3" + "@metamask/auto-changelog": "npm:^5.3.0" + "@metamask/eslint-config": "npm:^15.0.0" + "@metamask/eslint-config-nodejs": "npm:^15.0.0" + "@metamask/eslint-config-typescript": "npm:^15.0.0" + "@metamask/kernel-utils": "workspace:^" + "@metamask/ocap-kernel": "workspace:^" + "@ocap/repo-tools": "workspace:^" + "@ts-bridge/cli": "npm:^0.6.3" + "@ts-bridge/shims": "npm:^0.1.1" + "@typescript-eslint/eslint-plugin": "npm:^8.29.0" + "@typescript-eslint/parser": "npm:^8.29.0" + "@typescript-eslint/utils": "npm:^8.29.0" + "@vitest/eslint-plugin": "npm:^1.6.14" + depcheck: "npm:^1.4.7" + eslint: "npm:^9.23.0" + eslint-config-prettier: "npm:^10.1.1" + eslint-import-resolver-typescript: "npm:^4.3.1" + eslint-plugin-import-x: "npm:^4.10.0" + eslint-plugin-jsdoc: "npm:^50.6.9" + eslint-plugin-n: "npm:^17.17.0" + eslint-plugin-prettier: "npm:^5.2.6" + eslint-plugin-promise: "npm:^7.2.1" + prettier: "npm:^3.5.3" + rimraf: "npm:^6.0.1" + turbo: "npm:^2.9.1" + typedoc: "npm:^0.28.1" + typescript: "npm:~5.8.2" + typescript-eslint: "npm:^8.29.0" + vite: "npm:^8.0.6" + vitest: "npm:^4.1.3" + languageName: unknown + linkType: soft + "@ocap/omnium-gatherum@workspace:packages/omnium-gatherum": version: 0.0.0-use.local resolution: "@ocap/omnium-gatherum@workspace:packages/omnium-gatherum" From 9aaceba4ed8c0e0f7e34f797b75715f56b97d9a5 Mon Sep 17 00:00:00 2001 From: Chip Morningstar Date: Wed, 5 Aug 2026 14:44:10 -0700 Subject: [PATCH 10/24] fix(ocap-kernel,kernel-node-runtime): address review on IOListener lifetimes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three findings from review: - Closing a listener dropped its sockets but left every accepted connection's kref pinned, since release only ran from a connection's own `close()`. The listener service now tracks what it handed out and releases the outstanding ones when it closes. - A connection's `close()` signalled EOF and only then flushed the receive buffer, so a trailing partial line could still be handed to a later `read()` after EOF had been reported. Closing now discards buffered data first; a peer-initiated end still flushes, since that data arrived before the peer went away. - `releaseAnonymousKernelObject` now deletes the kernel object once nothing references it, rather than leaving it to `collectGarbage`, which skips kernel-owned objects (per review; a no-op at the current refcount baseline, correct once #1006 changes that). Peer disconnect still does not release on its own, and that is deliberate: the holder's c-list still names the kref, so releasing there would make a later call on the dropped reference reach `invokeKernelService`, find nothing registered, and throw — taking down the run loop. That is worse than a leak bounded by the listener's lifetime. Documented at the call site, pending #1006. Co-Authored-By: Claude Opus 4.7 --- .../src/io/socket-listener.test.ts | 24 ++++++++++++ .../src/io/socket-listener.ts | 8 ++++ .../ocap-kernel/src/KernelServiceManager.ts | 10 +++++ .../ocap-kernel/src/io/io-service.test.ts | 38 +++++++++++++++++++ packages/ocap-kernel/src/io/io-service.ts | 27 ++++++++++++- 5 files changed, 106 insertions(+), 1 deletion(-) diff --git a/packages/kernel-node-runtime/src/io/socket-listener.test.ts b/packages/kernel-node-runtime/src/io/socket-listener.test.ts index 68363158ea..4532181db9 100644 --- a/packages/kernel-node-runtime/src/io/socket-listener.test.ts +++ b/packages/kernel-node-runtime/src/io/socket-listener.test.ts @@ -310,6 +310,30 @@ describe('makeSocketIOListener', () => { expect(await channel.read()).toBeNull(); }); + it('discards buffered data on close rather than delivering it after EOF', async () => { + const socketPath = tempSocketPath(); + const listener = await makeTracked(socketPath); + + const client = await connectTracked(socketPath); + const channel = (await listener.accept()) as IOChannel; + + // A complete line plus a trailing fragment with no newline. + await writeLine(client, 'buffered'); + await new Promise((resolve, reject) => { + client.write('partial-no-newline', (error) => + error ? reject(error) : resolve(), + ); + }); + await settle(); + + await channel.close(); + + // Closing means the holder is done reading. Neither the queued line + // nor the trailing fragment may surface after EOF was signalled. + expect(await channel.read()).toBeNull(); + expect(await channel.read()).toBeNull(); + }); + it('throws on write after the channel is closed', async () => { const socketPath = tempSocketPath(); const listener = await makeTracked(socketPath); diff --git a/packages/kernel-node-runtime/src/io/socket-listener.ts b/packages/kernel-node-runtime/src/io/socket-listener.ts index 48beefa972..a3c34a9acd 100644 --- a/packages/kernel-node-runtime/src/io/socket-listener.ts +++ b/packages/kernel-node-runtime/src/io/socket-listener.ts @@ -135,6 +135,14 @@ function makeConnectionChannel( return; } closed = true; + // Discard anything still buffered before signalling EOF. Closing is + // the holder saying it is done reading, so a trailing partial line + // must not survive to be handed out by a later read() — that would + // deliver data after EOF. A peer-initiated end is the opposite case + // and does flush, since that data arrived before the peer went away. + lineQueue.length = 0; + buffer = ''; + ended = true; deliverEOF(); socket.destroy(); // `close` on the socket will fire handleEnd, but call it directly so diff --git a/packages/ocap-kernel/src/KernelServiceManager.ts b/packages/ocap-kernel/src/KernelServiceManager.ts index d55aeb2957..518f60e46c 100644 --- a/packages/ocap-kernel/src/KernelServiceManager.ts +++ b/packages/ocap-kernel/src/KernelServiceManager.ts @@ -153,6 +153,11 @@ export class KernelServiceManager { * unpinning it and removing it from the routing table. Idempotent, and * safe to call for a kref that was never registered. * + * The kernel object itself is deleted here once nothing references it, + * rather than being left to `collectGarbage`, which skips kernel-owned + * objects. With the current refcount baseline this branch does not fire; + * it is the correct place for the deletion once that changes (see #1006). + * * @param kref - The kref of the object to release. */ releaseAnonymousKernelObject(kref: KRef): void { @@ -160,6 +165,11 @@ export class KernelServiceManager { return; } this.#kernelStore.unpinObject(kref); + const { reachable, recognizable } = + this.#kernelStore.getObjectRefCount(kref); + if (reachable === 0 && recognizable === 0) { + this.#kernelStore.deleteKernelObject(kref); + } } /** diff --git a/packages/ocap-kernel/src/io/io-service.test.ts b/packages/ocap-kernel/src/io/io-service.test.ts index 4b760e2283..d3a4e9848a 100644 --- a/packages/ocap-kernel/src/io/io-service.test.ts +++ b/packages/ocap-kernel/src/io/io-service.test.ts @@ -343,4 +343,42 @@ describe('makeIOListenerService', () => { expect(underlying.close).toHaveBeenCalledOnce(); }); + + it('stops hosting outstanding connections when the listener closes', async () => { + const host = makeHost(); + const listener = makeIOListenerService( + 'io:s1:repl', + makeListener([makeChannel(), makeChannel()]), + makeConfig(), + host, + ) as ListenerFacet; + + await listener.accept(); + await listener.accept(); + // Neither connection was closed by its holder; closing the listener + // must still release them, or their krefs stay pinned for the life of + // the subcluster. + await listener.close(); + + expect(host.released).toStrictEqual(['ko1', 'ko2']); + }); + + it('does not release a connection twice when it was already closed', async () => { + const host = makeHost(); + const listener = makeIOListenerService( + 'io:s1:repl', + makeListener([makeChannel(), makeChannel()]), + makeConfig(), + host, + ) as ListenerFacet; + + await listener.accept(); + await listener.accept(); + const first = host.registered[0]?.connection as unknown as ConnectionFacet; + await first.close(); + + await listener.close(); + + expect(host.released).toStrictEqual(['ko1', 'ko2']); + }); }); diff --git a/packages/ocap-kernel/src/io/io-service.ts b/packages/ocap-kernel/src/io/io-service.ts index b61b61d0c0..09fe79374a 100644 --- a/packages/ocap-kernel/src/io/io-service.ts +++ b/packages/ocap-kernel/src/io/io-service.ts @@ -23,6 +23,19 @@ export type ConnectionHost = { * `direction` is enforced here rather than on the listener, since it is a * property of the data flow rather than of the point of contact. * + * Lifetime: the holder must `close()` a connection when finished with it. + * A peer disconnecting ends the underlying transport and makes `read()` + * report EOF, but does *not* by itself stop the kernel hosting this + * object, because the holder still has a live reference to it. Releasing + * on EOF instead would be actively worse than leaking: the vat's c-list + * still names the kref, so a subsequent call on the dropped reference + * would route to `invokeKernelService`, find nothing registered, and + * throw — which takes down the run loop. Until a vat dropping the + * reference is itself observable (see #1006), unreleased connections are + * bounded by their listener's lifetime: `close()` on the listener + * releases whatever it handed out, and `IOManager` releases the rest when + * the subcluster goes away. + * * @param name - The scoped connection name, used as the exo's interface * name (e.g. `io:s1:repl:c3`). * @param channel - The channel for this connection. @@ -92,6 +105,12 @@ export function makeIOListenerService( host: ConnectionHost, ): object { let nextConnectionId = 0; + /** + * Krefs of connections handed out and not yet released, so closing the + * listener stops hosting them too. Without this, closing a listener + * dropped its sockets but left every accepted connection's kref pinned. + */ + const hostedConnections = new Set(); return makeDefaultExo(name, { async accept(): Promise { @@ -113,16 +132,22 @@ export function makeIOListenerService( config, () => { if (hosted.kref) { + hostedConnections.delete(hosted.kref); host.release(hosted.kref); } }, ); hosted.kref = host.register(connection, connectionName); + hostedConnections.add(hosted.kref); return kslot(hosted.kref, connectionName); }, async close(): Promise { - return listener.close(); + await listener.close(); + for (const kref of [...hostedConnections]) { + hostedConnections.delete(kref); + host.release(kref); + } }, }); } From 2659ad42a7e6f734886af9d42dae06cb8fbea1c7 Mon Sep 17 00:00:00 2001 From: Chip Morningstar Date: Wed, 5 Aug 2026 15:17:21 -0700 Subject: [PATCH 11/24] fix(ocap-jsonrpc-vat): keep the wire protocol well-formed on edge cases MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three review findings, all cases where a client could be left with either no reply or a reply that is neither success nor error: - A void method returned `undefined`, which `JSON.stringify` drops, so the response carried neither `result` nor `error`. Normalized to `null`. Only `undefined` is substituted, so `0`, `''`, and `false` still report as themselves. - An unparseable request line was logged and dropped with no reply, so a client awaiting an answer on this request/reply socket waited forever. It now gets `PARSE_ERROR` with a null id, the id being unknowable from a line that would not parse. - A method may return a passable with no JSON form — a `bigint`, say — which `substituteRemotables` passes through untouched and which then throws in `JSON.stringify`. That was treated as a write failure and closed the connection. Encoding is now separate from writing, and an unencodable result yields an `INTERNAL_ERROR` reply instead. Co-Authored-By: Claude Opus 4.7 --- packages/ocap-jsonrpc-vat/src/bridge.test.ts | 54 ++++++++++++++++++++ packages/ocap-jsonrpc-vat/src/bridge.ts | 8 ++- packages/ocap-jsonrpc-vat/src/vat/index.ts | 53 +++++++++++++++++-- 3 files changed, 110 insertions(+), 5 deletions(-) diff --git a/packages/ocap-jsonrpc-vat/src/bridge.test.ts b/packages/ocap-jsonrpc-vat/src/bridge.test.ts index eac2cb78c7..0c8f416774 100644 --- a/packages/ocap-jsonrpc-vat/src/bridge.test.ts +++ b/packages/ocap-jsonrpc-vat/src/bridge.test.ts @@ -338,6 +338,60 @@ describe('send', () => { }, }); }); + + it('reports a void result as null so the response survives encoding', async () => { + const alpha = makeFake('alpha'); + const { bridge } = buildBridge({ + redeem: async () => alpha, + invoke: async () => undefined, + }); + await bridge.dispatch({ + jsonrpc: '2.0', + id: 1, + method: 'redeemURL', + params: { url: 'ocap:alpha' }, + }); + const response = await bridge.dispatch({ + jsonrpc: '2.0', + id: 2, + method: 'send', + params: { target: '@@j1', method: 'doNothing', args: [] }, + }); + + // `undefined` would be dropped by JSON.stringify, leaving a response + // with neither `result` nor `error` — valid as neither outcome. + expect(response).toStrictEqual({ jsonrpc: '2.0', id: 2, result: null }); + expect(JSON.parse(JSON.stringify(response))).toHaveProperty('result', null); + }); + + it.each([ + ['false', false], + ['zero', 0], + ['empty string', ''], + ])( + 'preserves a falsy %s result rather than nulling it', + async (_l, value) => { + const alpha = makeFake('alpha'); + const { bridge } = buildBridge({ + redeem: async () => alpha, + invoke: async () => value, + }); + await bridge.dispatch({ + jsonrpc: '2.0', + id: 1, + method: 'redeemURL', + params: { url: 'ocap:alpha' }, + }); + const response = await bridge.dispatch({ + jsonrpc: '2.0', + id: 2, + method: 'send', + params: { target: '@@j1', method: 'give', args: [] }, + }); + + expect(response).toStrictEqual({ jsonrpc: '2.0', id: 2, result: value }); + }, + ); }); describe('resetSession', () => { diff --git a/packages/ocap-jsonrpc-vat/src/bridge.ts b/packages/ocap-jsonrpc-vat/src/bridge.ts index 82d12c1633..a40d853298 100644 --- a/packages/ocap-jsonrpc-vat/src/bridge.ts +++ b/packages/ocap-jsonrpc-vat/src/bridge.ts @@ -293,12 +293,18 @@ function extractId(value: unknown): JsonRpcId { /** * Build a JSON-RPC success response. * + * A void method yields `undefined`, which `JSON.stringify` drops entirely — + * producing a response carrying neither `result` nor `error`, which is + * well-formed as neither outcome under JSON-RPC 2.0. Normalizing to `null` + * keeps the success shape intact. Only `undefined` is substituted, so + * falsy results like `0`, `''`, and `false` are reported as they are. + * * @param id - The request id to echo. * @param result - The result payload. * @returns The response envelope. */ function successResponse(id: JsonRpcId, result: unknown): JsonRpcResponse { - return { jsonrpc: '2.0', id, result }; + return { jsonrpc: '2.0', id, result: result ?? null }; } /** diff --git a/packages/ocap-jsonrpc-vat/src/vat/index.ts b/packages/ocap-jsonrpc-vat/src/vat/index.ts index 8ec7e9d9e2..5bc2a65efc 100644 --- a/packages/ocap-jsonrpc-vat/src/vat/index.ts +++ b/packages/ocap-jsonrpc-vat/src/vat/index.ts @@ -123,12 +123,57 @@ export function buildRootObject( try { request = JSON.parse(line); } catch (error) { - log('failed to parse request line as JSON; dropping:', error); - return 'ok'; + // Reply rather than dropping: this is a request/reply socket, so a + // client awaiting an answer would otherwise wait forever. The id is + // unknowable from an unparseable line, which is exactly the case + // JSON-RPC 2.0 covers with a null id. + log('failed to parse request line as JSON:', error); + return await respond(connection, { + jsonrpc: '2.0', + id: null, + error: { + code: JSON_RPC_ERROR.PARSE_ERROR, + message: 'request line is not valid JSON', + }, + }); + } + return await respond(connection, await dispatch(request)); + } + + /** + * Encode and write one response. + * + * Encoding can fail even when dispatch succeeded, because a method may + * return a passable that has no JSON form — a `bigint`, say — which + * `substituteRemotables` passes through untouched. Sending an error in + * that case keeps the exchange to one reply per request; treating it as a + * write failure would drop the connection and leave the client waiting. + * + * @param connection - The connection to write to. + * @param response - The response to encode and send. + * @returns 'ok' if the response was written, 'closed' if the connection + * could not be written to. + */ + async function respond( + connection: IOConnection, + response: JsonRpcResponse, + ): Promise<'ok' | 'closed'> { + let encoded: string; + try { + encoded = JSON.stringify(response); + } catch (error) { + log('failed to encode response:', error); + encoded = JSON.stringify({ + jsonrpc: '2.0', + id: response.id, + error: { + code: JSON_RPC_ERROR.INTERNAL_ERROR, + message: 'result could not be encoded as JSON', + }, + }); } - const response = await dispatch(request); try { - await E(connection).write(JSON.stringify(response)); + await E(connection).write(encoded); } catch (error) { log('failed to write response:', error); return 'closed'; From 2c72192c42d68a216bbd78c0c38c430beabc240c Mon Sep 17 00:00:00 2001 From: Chip Morningstar Date: Wed, 5 Aug 2026 15:26:29 -0700 Subject: [PATCH 12/24] fix(kernel-node-runtime): report a holder-initiated close to the listener MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to review on the previous commit: setting `ended` inside `close()` made `handleEnd` return early and skip `onClosed`, so a channel closed by its holder stayed registered with the listener — a long-lived listener would accumulate every session it ever served. The flush-or-discard decision now lives in `handleEnd` and is keyed on `closed`, so both paths reach `onClosed` exactly once while a trailing partial line is still flushed for a peer-initiated end and discarded for a holder close. `makeConnectionChannel` is exported so this is testable directly; the package's public surface is unchanged, since `io/index.ts` does not re-export it. Co-Authored-By: Claude Opus 4.7 --- .../src/io/socket-listener.test.ts | 87 ++++++++++++++++++- .../src/io/socket-listener.ts | 35 ++++---- 2 files changed, 104 insertions(+), 18 deletions(-) diff --git a/packages/kernel-node-runtime/src/io/socket-listener.test.ts b/packages/kernel-node-runtime/src/io/socket-listener.test.ts index 4532181db9..9c201cba68 100644 --- a/packages/kernel-node-runtime/src/io/socket-listener.test.ts +++ b/packages/kernel-node-runtime/src/io/socket-listener.test.ts @@ -1,11 +1,15 @@ import type { IOChannel, IOListener } from '@metamask/ocap-kernel'; +import { EventEmitter } from 'node:events'; import fs from 'node:fs/promises'; import * as net from 'node:net'; import * as os from 'node:os'; import * as path from 'node:path'; -import { describe, it, expect, afterEach } from 'vitest'; +import { describe, it, expect, afterEach, vi } from 'vitest'; -import { makeSocketIOListener } from './socket-listener.ts'; +import { + makeConnectionChannel, + makeSocketIOListener, +} from './socket-listener.ts'; function tempSocketPath(): string { return path.join( @@ -423,3 +427,82 @@ describe('makeSocketIOListener', () => { expect(await fileExists(socketPath)).toBe(true); }); }); + +describe('makeConnectionChannel', () => { + /** + * A minimal stand-in for a connected socket: enough of the surface for + * the channel to attach handlers, and emitters so a test can drive the + * peer side directly. + * + * @returns The fake socket. + */ + function makeFakeSocket(): net.Socket { + const emitter = new EventEmitter(); + return Object.assign(emitter, { + destroy: () => undefined, + write: () => true, + }) as unknown as net.Socket; + } + + it('reports the connection closed when the holder closes it', async () => { + const onClosed = vi.fn(); + const channel = makeConnectionChannel('c1', makeFakeSocket(), onClosed); + + await channel.close(); + + // Without this the listener keeps the channel registered forever, so a + // long-lived listener accumulates every session it ever served. + expect(onClosed).toHaveBeenCalledOnce(); + }); + + it('reports the connection closed when the peer ends it', () => { + const onClosed = vi.fn(); + const socket = makeFakeSocket(); + makeConnectionChannel('c1', socket, onClosed); + + socket.emit('end'); + + expect(onClosed).toHaveBeenCalledOnce(); + }); + + it('reports closed only once across peer end and holder close', async () => { + const onClosed = vi.fn(); + const socket = makeFakeSocket(); + const channel = makeConnectionChannel('c1', socket, onClosed); + + socket.emit('end'); + await channel.close(); + socket.emit('close'); + + expect(onClosed).toHaveBeenCalledOnce(); + }); + + it('flushes a trailing partial line when the peer ends', async () => { + const channel = makeConnectionChannel( + 'c1', + (() => { + const peer = makeFakeSocket(); + setImmediate(() => { + peer.emit('data', Buffer.from('no-newline-here')); + peer.emit('end'); + }); + return peer; + })(), + vi.fn(), + ); + + // Data that arrived before the peer went away is still owed to the reader. + expect(await channel.read()).toBe('no-newline-here'); + expect(await channel.read()).toBeNull(); + }); + + it('discards a trailing partial line when the holder closes', async () => { + const socket = makeFakeSocket(); + const channel = makeConnectionChannel('c1', socket, vi.fn()); + + socket.emit('data', Buffer.from('no-newline-here')); + await channel.close(); + + expect(await channel.read()).toBeNull(); + }); +}); diff --git a/packages/kernel-node-runtime/src/io/socket-listener.ts b/packages/kernel-node-runtime/src/io/socket-listener.ts index a3c34a9acd..a9ab3f5c11 100644 --- a/packages/kernel-node-runtime/src/io/socket-listener.ts +++ b/packages/kernel-node-runtime/src/io/socket-listener.ts @@ -25,7 +25,7 @@ type PendingAcceptor = { * because the peer went away or because `close()` was called. * @returns The channel for this connection. */ -function makeConnectionChannel( +export function makeConnectionChannel( name: string, socket: net.Socket, onClosed: () => void, @@ -77,20 +77,24 @@ function makeConnectionChannel( } /** - * Handle the peer going away. Flushes any trailing partial line, then - * reports EOF. Unlike a shared channel, there is no ambiguity about - * whose session ended: this channel serves exactly one peer, so the - * end of the socket is the end of the channel. + * Handle the channel finishing, from either end. Unlike a shared channel + * there is no ambiguity about whose session ended: this channel serves + * exactly one peer, so the end of the socket is the end of the channel. + * + * A trailing partial line is flushed only when the peer ended things, + * because that data arrived before the peer went away. When the holder + * called `close()` it is discarded instead — EOF has already been + * reported, and handing data over afterwards would contradict it. */ function handleEnd(): void { if (ended) { return; } ended = true; - buffer += decoder.end(); - if (buffer.length > 0) { - deliverLine(buffer); - buffer = ''; + const trailing = buffer + decoder.end(); + buffer = ''; + if (!closed && trailing.length > 0) { + deliverLine(trailing); } deliverEOF(); onClosed(); @@ -135,14 +139,13 @@ function makeConnectionChannel( return; } closed = true; - // Discard anything still buffered before signalling EOF. Closing is - // the holder saying it is done reading, so a trailing partial line - // must not survive to be handed out by a later read() — that would - // deliver data after EOF. A peer-initiated end is the opposite case - // and does flush, since that data arrived before the peer went away. + // Drop lines already queued: the holder is done reading, so nothing + // buffered may surface from a later read() once EOF is reported. + // `handleEnd` discards the trailing fragment for the same reason, + // keyed on `closed`. Deliberately not setting `ended` here — that + // would make `handleEnd` return early and skip `onClosed`, leaving + // this channel registered with the listener for good. lineQueue.length = 0; - buffer = ''; - ended = true; deliverEOF(); socket.destroy(); // `close` on the socket will fire handleEnd, but call it directly so From e68a80faacc570c4ec7b0a308fa9023541d59fa2 Mon Sep 17 00:00:00 2001 From: Chip Morningstar Date: Wed, 5 Aug 2026 15:30:50 -0700 Subject: [PATCH 13/24] chore(ocap-jsonrpc-vat): correct the stale vitest project name The project was still called `llm-mediator-vat`, the package's name before it was renamed, so its tests were mislabelled in monorepo output and in `--project` filters. Co-Authored-By: Claude Opus 4.7 --- packages/ocap-jsonrpc-vat/vitest.config.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ocap-jsonrpc-vat/vitest.config.ts b/packages/ocap-jsonrpc-vat/vitest.config.ts index 7606c3d4d4..82bd3b0077 100644 --- a/packages/ocap-jsonrpc-vat/vitest.config.ts +++ b/packages/ocap-jsonrpc-vat/vitest.config.ts @@ -10,7 +10,7 @@ export default defineConfig((args) => { defaultConfig, defineProject({ test: { - name: 'llm-mediator-vat', + name: 'ocap-jsonrpc-vat', setupFiles: [ fileURLToPath( import.meta.resolve('@ocap/repo-tools/test-utils/mock-endoify'), From e23632596fa848615de3c665d08327238c52a719 Mon Sep 17 00:00:00 2001 From: Chip Morningstar Date: Wed, 5 Aug 2026 15:32:03 -0700 Subject: [PATCH 14/24] docs(ocap-kernel): the run-queue bug is not startup-only MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per review: `rollbackCrank` also invalidates the length cache, and a rollback is normally followed straight away by enqueueing an error or termination message — which is precisely the sequence that trips the bug. That path is more likely in practice than the startup one the entry originally described. Co-Authored-By: Claude Opus 4.7 --- packages/ocap-kernel/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ocap-kernel/CHANGELOG.md b/packages/ocap-kernel/CHANGELOG.md index e8db1530f1..5c7cdaf23f 100644 --- a/packages/ocap-kernel/CHANGELOG.md +++ b/packages/ocap-kernel/CHANGELOG.md @@ -41,7 +41,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed -- The kernel run queue no longer strands messages after a restart. `runQueueLengthCache` uses a negative value to mean "unknown, re-read from the database", but `enqueueRun`/`dequeueRun` adjusted it arithmetically without materializing it first — so an enqueue while the cache held its startup value of `-1` produced `0` for a queue that actually held an item, and because `0` is not negative it was never re-read. The run loop then saw an empty queue, went to sleep, and stranded everything queued behind it, with no error and no log. The run loop is also now woken by any non-empty queue rather than only by the empty-to-one transition, so a drifted count cannot silently lose the wakeup ([#1007](https://github.com/MetaMask/ocap-kernel/pull/1007)) +- The kernel run queue no longer strands messages, going quiet with no error, no log, and no crash. `runQueueLengthCache` uses a negative value to mean "unknown, re-read from the database", but `enqueueRun`/`dequeueRun` adjusted it arithmetically without materializing it first — so an enqueue while the cache held `-1` produced `0` for a queue that actually held an item, and because `0` is not negative it was never re-read again. The run loop then saw an empty queue, went to sleep, and stranded everything behind it. Two paths reach that `-1`: kernel startup, and `rollbackCrank`, which invalidates the cache because a rollback may have restored dequeued items. The rollback path is the more likely of the two in practice, since a rollback is normally followed immediately by enqueueing an error or termination message. The run loop is also now woken by any non-empty queue rather than only by the empty-to-one transition, so a drifted count cannot silently lose the wakeup either ([#1007](https://github.com/MetaMask/ocap-kernel/pull/1007)) - Deserialize CapData rejections in `Kernel.queueMessage` so vat errors surface as plain `Error` objects to all callers ([#928](https://github.com/MetaMask/ocap-kernel/pull/928)) - Detect peer restart across receiver state loss so the receiving kernel no longer silently drops a restarted peer's `seq=1` messages ([#948](https://github.com/MetaMask/ocap-kernel/pull/948)) - Persist the peer's last-observed incarnation and compare it on every successful handshake; on a detected restart, clear the peer's c-list contributions and reject the promises it was deciding before the new incarnation reuses any erefs From 65474ed7551049edf67c7492ee1d3cd4933a39a8 Mon Sep 17 00:00:00 2001 From: Chip Morningstar Date: Wed, 5 Aug 2026 16:33:21 -0700 Subject: [PATCH 15/24] fix(kernel-node-runtime): ignore socket data arriving after the channel ends MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Node can still emit 'data' after `socket.destroy()`, and `handleData` checked neither flag. A late chunk therefore refilled the queue that `close()` had just cleared, and since `read()` drains the queue before consulting the flags, it would hand that line out after EOF had been reported. Data that arrived before the end is unaffected — it is already queued and stays readable, which is what a peer-initiated end owes its reader. Both halves are now covered by tests. Co-Authored-By: Claude Opus 4.7 --- .../src/io/socket-listener.test.ts | 24 +++++++++++++++++++ .../src/io/socket-listener.ts | 9 +++++++ 2 files changed, 33 insertions(+) diff --git a/packages/kernel-node-runtime/src/io/socket-listener.test.ts b/packages/kernel-node-runtime/src/io/socket-listener.test.ts index 9c201cba68..101289f1a1 100644 --- a/packages/kernel-node-runtime/src/io/socket-listener.test.ts +++ b/packages/kernel-node-runtime/src/io/socket-listener.test.ts @@ -496,6 +496,30 @@ describe('makeConnectionChannel', () => { expect(await channel.read()).toBeNull(); }); + it('ignores data that arrives after close', async () => { + const socket = makeFakeSocket(); + const channel = makeConnectionChannel('c1', socket, () => undefined); + + await channel.close(); + // Node can still emit 'data' after destroy(); a late chunk must not + // refill the queue that close() cleared. + socket.emit('data', Buffer.from('too-late\n')); + + expect(await channel.read()).toBeNull(); + }); + + it('still delivers data that arrived before a peer end', async () => { + const socket = makeFakeSocket(); + const channel = makeConnectionChannel('c1', socket, () => undefined); + + socket.emit('data', Buffer.from('in-time\n')); + socket.emit('end'); + socket.emit('data', Buffer.from('too-late\n')); + + expect(await channel.read()).toBe('in-time'); + expect(await channel.read()).toBeNull(); + }); + it('discards a trailing partial line when the holder closes', async () => { const socket = makeFakeSocket(); const channel = makeConnectionChannel('c1', socket, vi.fn()); diff --git a/packages/kernel-node-runtime/src/io/socket-listener.ts b/packages/kernel-node-runtime/src/io/socket-listener.ts index a9ab3f5c11..163590953d 100644 --- a/packages/kernel-node-runtime/src/io/socket-listener.ts +++ b/packages/kernel-node-runtime/src/io/socket-listener.ts @@ -66,6 +66,15 @@ export function makeConnectionChannel( * @param data - The raw data from the socket. */ function handleData(data: Buffer): void { + if (ended || closed) { + // Node can still emit 'data' after `socket.destroy()`. Accepting a + // late chunk would refill the queue that `close()` just cleared, and + // a subsequent `read()` would hand it out even though EOF has already + // been reported. Data that arrived *before* the end is unaffected: it + // is already queued and stays readable, which is what a peer-initiated + // end owes its reader. + return; + } buffer += decoder.write(data); let newlineIndex = buffer.indexOf('\n'); while (newlineIndex !== -1) { From 547054a8f9bf5b7fad280ddbc8ae0d7e7dc93de1 Mon Sep 17 00:00:00 2001 From: Chip Morningstar Date: Wed, 5 Aug 2026 16:39:46 -0700 Subject: [PATCH 16/24] fix(ocap-jsonrpc-vat): require a request id instead of accepting notifications MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `isJsonRpcRequest` accepted a request with no `id` — a JSON-RPC notification — but `dispatch` always produces a response and the vat always writes it. On a persistent line-delimited socket that extra reply sits in the client's buffer and is read as the answer to some later request, scrambling request/response pairing from then on. Requiring an id keeps the invariant that every line in gets exactly one line back, which is what keeps the stream in step. Notifications would be pointless here anyway, since both methods exist to return a value. It also makes the type predicate honest: `JsonRpcRequest.id` is `JsonRpcId`, which does not include `undefined`. An explicit null id is still accepted, being legal in a request; only an absent one is rejected. Co-Authored-By: Claude Opus 4.7 --- packages/ocap-jsonrpc-vat/src/bridge.test.ts | 38 ++++++++++++++++++++ packages/ocap-jsonrpc-vat/src/bridge.ts | 13 +++++-- 2 files changed, 49 insertions(+), 2 deletions(-) diff --git a/packages/ocap-jsonrpc-vat/src/bridge.test.ts b/packages/ocap-jsonrpc-vat/src/bridge.test.ts index 0c8f416774..b852fc54eb 100644 --- a/packages/ocap-jsonrpc-vat/src/bridge.test.ts +++ b/packages/ocap-jsonrpc-vat/src/bridge.test.ts @@ -73,6 +73,44 @@ describe('dispatch: request validation', () => { }); }); + it('rejects a request with no id rather than treating it as a notification', async () => { + const { bridge } = buildBridge(); + const response = await bridge.dispatch({ + jsonrpc: '2.0', + method: 'redeemURL', + params: { url: 'ocap:alpha' }, + }); + + // Every line in gets exactly one line back. Serving notifications + // would make some lines answerable and others not, which desynchronizes + // a persistent line-delimited stream for good. + expect(response).toStrictEqual({ + jsonrpc: '2.0', + id: null, + error: { + code: JSON_RPC_ERROR.INVALID_REQUEST, + message: 'not a well-formed JSON-RPC 2.0 request', + }, + }); + }); + + it('accepts an explicit null id', async () => { + const { bridge } = buildBridge({ redeem: async () => makeFake('alpha') }); + const response = await bridge.dispatch({ + jsonrpc: '2.0', + id: null, + method: 'redeemURL', + params: { url: 'ocap:alpha' }, + }); + + // A null id is legal in a request; only an absent one is a notification. + expect(response).toStrictEqual({ + jsonrpc: '2.0', + id: null, + result: '@@j1', + }); + }); + it('rejects an unknown method', async () => { const { bridge } = buildBridge(); const response = await bridge.dispatch({ diff --git a/packages/ocap-jsonrpc-vat/src/bridge.ts b/packages/ocap-jsonrpc-vat/src/bridge.ts index a40d853298..8793ed3ee8 100644 --- a/packages/ocap-jsonrpc-vat/src/bridge.ts +++ b/packages/ocap-jsonrpc-vat/src/bridge.ts @@ -261,11 +261,20 @@ function isJsonRpcRequest(value: unknown): value is JsonRpcRequest { if (bag.jsonrpc !== '2.0' || typeof bag.method !== 'string') { return false; } + // An `id` is required: a missing one denotes a JSON-RPC notification, + // which this vat does not serve. Every line in gets exactly one line + // back, and that invariant is what keeps a persistent line-delimited + // stream in step — an unanswered request or an unexpected extra reply + // desynchronizes it permanently, with the client reading each answer as + // the response to some later request. Notifications would also be + // pointless here, since both methods exist to return a value. + // + // Rejecting also makes the predicate honest: `JsonRpcRequest.id` is + // `JsonRpcId`, which does not include `undefined`. if ( bag.id !== null && typeof bag.id !== 'number' && - typeof bag.id !== 'string' && - bag.id !== undefined + typeof bag.id !== 'string' ) { return false; } From f18ca4107c6f990a4727ac86abe91fd9de6d8d47 Mon Sep 17 00:00:00 2001 From: Chip Morningstar Date: Thu, 6 Aug 2026 14:29:30 -0700 Subject: [PATCH 17/24] fix(ocap-kernel): sweep anonymous kernel objects abandoned by a previous incarnation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per review. `registerAnonymousKernelObject` recorded its object only in the in-memory routing table, but `initKernelObject` and `pinObject` both write to the store — so an anonymous object survived a restart while its routing entry did not. Unlike a named service there is no name to re-register it under, leaving it unreachable but still pinned, accumulating with every restart. Worse, it stayed owned by `'kernel'`, so a delivery to a stale connection kref would reach `invokeKernelService`, find nothing registered, throw, and kill the run loop — the same failure this PR's other fix exists to prevent. Anonymous objects are now recorded in the store and swept at init, before the run queue starts so nothing can be delivered to a stale kref in the meantime. These host things that cannot outlive the process — an accepted socket connection, say — so a survivor is unambiguously garbage. Co-Authored-By: Claude Opus 4.7 --- packages/ocap-kernel/CHANGELOG.md | 1 + packages/ocap-kernel/src/Kernel.ts | 15 ++++++++ .../src/KernelServiceManager.test.ts | 37 +++++++++++++++++++ .../ocap-kernel/src/KernelServiceManager.ts | 37 +++++++++++++++++++ packages/ocap-kernel/src/store/index.test.ts | 3 ++ packages/ocap-kernel/src/store/index.ts | 29 +++++++++++++++ 6 files changed, 122 insertions(+) diff --git a/packages/ocap-kernel/CHANGELOG.md b/packages/ocap-kernel/CHANGELOG.md index 130cec6507..98961456dd 100644 --- a/packages/ocap-kernel/CHANGELOG.md +++ b/packages/ocap-kernel/CHANGELOG.md @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - Add `IOListener`, an endpoint peers connect to that yields one `IOChannel` per connection via `accept()`, replacing the previous one-client-at-a-time channel. Each accepted connection is a distinct object, so holding one conveys no way to reach another, and `direction` is enforced per connection. `accept()` resolves `null` once the listener is closed so an accept loop can terminate rather than hang ([#1007](https://github.com/MetaMask/ocap-kernel/pull/1007)) +- Anonymous kernel-hosted objects are recorded persistently and swept at kernel init, so one abandoned by a previous incarnation is neither left pinned forever nor able to take down the run loop when something is delivered to it — an anonymous object has no name to be re-registered under on boot, unlike a named service ([#1007](https://github.com/MetaMask/ocap-kernel/pull/1007)) - Add `KernelServiceManager.registerAnonymousKernelObject()` / `releaseAnonymousKernelObject()`, which make a kernel-hosted object routable by kref without entering it in the service-name index, so it has no name in the global service namespace and cannot be requested via a cluster config's `services` list. Used to host accepted IO connections, whose authority comes from holding the reference ([#1007](https://github.com/MetaMask/ocap-kernel/pull/1007)) - Report run loop health in `KernelStatus.runLoop` (`{ state: 'idle' | 'running' }` or `{ state: 'failed', error, detail }`), exporting `RunLoopStatus`, `RunLoopStatusStruct`, and `OnRunLoopFailure` ([#1005](https://github.com/MetaMask/ocap-kernel/pull/1005)) - `idle` means never started; a loop parked on an empty queue reports `running` diff --git a/packages/ocap-kernel/src/Kernel.ts b/packages/ocap-kernel/src/Kernel.ts index c0ea061b1d..f99bebfd45 100644 --- a/packages/ocap-kernel/src/Kernel.ts +++ b/packages/ocap-kernel/src/Kernel.ts @@ -294,6 +294,21 @@ export class Kernel { // the run queue has no selective removal capability. this.provideFacet(); + // Discard anonymous kernel objects from a previous incarnation. They + // host things that cannot outlive the process — an accepted socket + // connection, say — and unlike a named service there is no name to + // re-register one under, so a survivor is unreachable but still pinned. + // Same hazard the facet registration above guards against: a delivery to + // one would find nothing registered and kill the run queue. Swept before + // the queue starts for exactly that reason. + const abandoned = + this.#kernelServiceManager.releaseAbandonedAnonymousKernelObjects(); + if (abandoned > 0) { + this.#logger.info( + `Released ${abandoned} anonymous kernel object(s) abandoned by a previous incarnation`, + ); + } + // Restore persisted system subclusters and delete ones that no // longer have a config, to ensure that orphaned vats aren't started this.#subclusterManager.initSystemSubclusters(configs); diff --git a/packages/ocap-kernel/src/KernelServiceManager.test.ts b/packages/ocap-kernel/src/KernelServiceManager.test.ts index 3153297c3d..ffbffc9792 100644 --- a/packages/ocap-kernel/src/KernelServiceManager.test.ts +++ b/packages/ocap-kernel/src/KernelServiceManager.test.ts @@ -551,6 +551,8 @@ describe('KernelServiceManager', () => { // The whole point: absent from the global name namespace, so no // string can be used to ask for it. expect(serviceManager.getKernelService('io-connection')).toBeUndefined(); + // Recorded persistently so a later incarnation can sweep it. + expect(kernelStore.getAnonymousKernelObjects()).toStrictEqual([kref]); }); it('allows the same label for distinct objects', () => { @@ -603,6 +605,41 @@ describe('KernelServiceManager', () => { }); }); + describe('releaseAbandonedAnonymousKernelObjects', () => { + it('discards objects recorded by a previous incarnation', () => { + // Simulate a restart: the krefs are still recorded in the store, but + // the in-memory routing table starts empty. + const stale = kernelStore.initKernelObject('kernel'); + kernelStore.pinObject(stale); + kernelStore.addAnonymousKernelObject(stale); + + const fresh = new KernelServiceManager({ + kernelStore, + kernelQueue: mockKernelQueue, + logger, + }); + + expect(fresh.releaseAbandonedAnonymousKernelObjects()).toBe(1); + expect(kernelStore.isObjectPinned(stale)).toBe(false); + expect(kernelStore.getAnonymousKernelObjects()).toStrictEqual([]); + }); + + it('leaves objects hosted by the current incarnation alone', () => { + const live = serviceManager.registerAnonymousKernelObject( + { ping: () => 'pong' }, + 'io-connection', + ); + + expect(serviceManager.releaseAbandonedAnonymousKernelObjects()).toBe(0); + expect(serviceManager.isKernelService(live)).toBe(true); + expect(kernelStore.isObjectPinned(live)).toBe(true); + }); + + it('reports nothing to do when none were recorded', () => { + expect(serviceManager.releaseAbandonedAnonymousKernelObjects()).toBe(0); + }); + }); + describe('releaseAnonymousKernelObject', () => { it('removes the object from routing and unpins it', () => { const kref = serviceManager.registerAnonymousKernelObject( diff --git a/packages/ocap-kernel/src/KernelServiceManager.ts b/packages/ocap-kernel/src/KernelServiceManager.ts index 518f60e46c..c0ad7262a9 100644 --- a/packages/ocap-kernel/src/KernelServiceManager.ts +++ b/packages/ocap-kernel/src/KernelServiceManager.ts @@ -139,6 +139,11 @@ export class KernelServiceManager { registerAnonymousKernelObject(service: object, label: string): KRef { const kref = this.#kernelStore.initKernelObject('kernel'); this.#kernelStore.pinObject(kref); + // Recorded persistently so `releaseAbandonedAnonymousKernelObjects` can + // find it after a restart. The routing entry below is in-memory only, + // and an anonymous object has no name to be re-registered under, so one + // that outlives its incarnation is unreachable yet still pinned. + this.#kernelStore.addAnonymousKernelObject(kref); this.#kernelServicesByObject.set(kref, { name: label, kref, @@ -148,6 +153,37 @@ export class KernelServiceManager { return kref; } + /** + * Discard anonymous kernel objects left behind by a previous incarnation. + * + * These exist to host things that cannot outlive the process — an accepted + * socket connection, say — so any that survived a restart are garbage. They + * are also actively harmful if left: still pinned, so they accumulate with + * every restart, and still owned by `'kernel'`, so a delivery to one would + * reach `invokeKernelService`, find nothing registered, throw, and take the + * run loop down with it. + * + * Must run before the run queue starts, so nothing can be delivered to a + * stale kref in the window before the sweep. + * + * @returns The number of objects discarded. + */ + releaseAbandonedAnonymousKernelObjects(): number { + const abandoned = this.#kernelStore + .getAnonymousKernelObjects() + .filter((kref) => !this.#kernelServicesByObject.has(kref)); + for (const kref of abandoned) { + this.#kernelStore.unpinObject(kref); + const { reachable, recognizable } = + this.#kernelStore.getObjectRefCount(kref); + if (reachable === 0 && recognizable === 0) { + this.#kernelStore.deleteKernelObject(kref); + } + this.#kernelStore.removeAnonymousKernelObject(kref); + } + return abandoned.length; + } + /** * Release an object registered with `registerAnonymousKernelObject`, * unpinning it and removing it from the routing table. Idempotent, and @@ -170,6 +206,7 @@ export class KernelServiceManager { if (reachable === 0 && recognizable === 0) { this.#kernelStore.deleteKernelObject(kref); } + this.#kernelStore.removeAnonymousKernelObject(kref); } /** diff --git a/packages/ocap-kernel/src/store/index.test.ts b/packages/ocap-kernel/src/store/index.test.ts index 3db43c3e31..58fefc80c3 100644 --- a/packages/ocap-kernel/src/store/index.test.ts +++ b/packages/ocap-kernel/src/store/index.test.ts @@ -40,6 +40,7 @@ describe('kernel store', () => { it('has all the expected parts', () => { const ks = makeKernelStore(mockKernelDatabase); expect(Object.keys(ks).sort()).toStrictEqual([ + 'addAnonymousKernelObject', 'addCListEntry', 'addGCActions', 'addPromiseSubscriber', @@ -86,6 +87,7 @@ describe('kernel store', () => { 'getAllRemoteRecords', 'getAllSystemSubclusterMappings', 'getAllVatRecords', + 'getAnonymousKernelObjects', 'getGCActions', 'getImporters', 'getKernelPromise', @@ -149,6 +151,7 @@ describe('kernel store', () => { 'recordLastActiveTime', 'releaseAllSavepoints', 'releaseSavepoint', + 'removeAnonymousKernelObject', 'removeVatFromSubcluster', 'reset', 'resolveKernelPromise', diff --git a/packages/ocap-kernel/src/store/index.ts b/packages/ocap-kernel/src/store/index.ts index 144b3a6c48..5c8f49fc3d 100644 --- a/packages/ocap-kernel/src/store/index.ts +++ b/packages/ocap-kernel/src/store/index.ts @@ -334,6 +334,35 @@ export function makeKernelStore(kdb: KernelDatabase, logger?: Logger) { deleteKernelServiceKref(name: string): void { kv.delete(`kernelService.${name}`); }, + + // Anonymous kernel-hosted objects + // + // Recorded so they can be swept at kernel init. Unlike a named service, + // an anonymous object has no name to be re-registered under on boot, so + // one surviving a restart is unreachable but still pinned — and a + // delivery to it would find nothing registered and kill the run loop. + // They are used for things like accepted IO connections, which cannot + // outlive the process anyway. + getAnonymousKernelObjects(): KRef[] { + const raw = kv.get('anonymousKernelObjects'); + return raw ? (raw.split(',') as KRef[]) : []; + }, + addAnonymousKernelObject(kref: KRef): void { + const krefs = new Set(this.getAnonymousKernelObjects()); + krefs.add(kref); + kv.set('anonymousKernelObjects', [...krefs].sort().join(',')); + }, + removeAnonymousKernelObject(kref: KRef): void { + const krefs = new Set(this.getAnonymousKernelObjects()); + if (!krefs.delete(kref)) { + return; + } + if (krefs.size === 0) { + kv.delete('anonymousKernelObjects'); + } else { + kv.set('anonymousKernelObjects', [...krefs].sort().join(',')); + } + }, }); } From 6dee3a5d1e45754eff39c18720102f4fe8070a4c Mon Sep 17 00:00:00 2001 From: Chip Morningstar Date: Thu, 6 Aug 2026 14:37:43 -0700 Subject: [PATCH 18/24] fix(ocap-jsonrpc-vat): refuse to serialize unsettled promises A promise has no own enumerable properties, so the response walker turned one into `{}` and JSON.stringify accepted it. --- packages/ocap-jsonrpc-vat/CHANGELOG.md | 5 +-- .../ocap-jsonrpc-vat/src/json-rpc.test.ts | 18 ++++++++++ packages/ocap-jsonrpc-vat/src/json-rpc.ts | 33 +++++++++++++++++++ 3 files changed, 54 insertions(+), 2 deletions(-) diff --git a/packages/ocap-jsonrpc-vat/CHANGELOG.md b/packages/ocap-jsonrpc-vat/CHANGELOG.md index ebb4c85a15..04049d6f9f 100644 --- a/packages/ocap-jsonrpc-vat/CHANGELOG.md +++ b/packages/ocap-jsonrpc-vat/CHANGELOG.md @@ -10,8 +10,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - Initial release: vat serving a line-delimited JSON-RPC 2.0 protocol on a Unix-domain-socket `IOService` endowment - - `redeemURL(url)` redeems an OCAP URL through the kernel's `ocapURLRedemptionService` and returns a sigil name of the form `@@o` referring to the resulting live reference - - `send(target, method, args)` invokes `E(target)[method](...args)` with `@@o` markers in `args` expanded to their live references and any remotable in the result substituted for its sigil name + - `redeemURL(url)` redeems an OCAP URL through the kernel's `ocapURLRedemptionService` and returns a sigil name of the form `@@j` referring to the resulting live reference + - `send(target, method, args)` invokes `E(target)[method](...args)` with `@@j` markers in `args` expanded to their live references and any remotable in the result substituted for its sigil name + - A result containing an unsettled promise is refused with an internal error rather than serialized: a promise has no own enumerable properties, so it would otherwise become `{}` in a response that `JSON.stringify` accepts, silently handing the client a success payload with the value missing - Session state is in-memory only and resets on socket disconnect [Unreleased]: https://github.com/MetaMask/ocap-kernel/ diff --git a/packages/ocap-jsonrpc-vat/src/json-rpc.test.ts b/packages/ocap-jsonrpc-vat/src/json-rpc.test.ts index 06f2144b64..fd126c2973 100644 --- a/packages/ocap-jsonrpc-vat/src/json-rpc.test.ts +++ b/packages/ocap-jsonrpc-vat/src/json-rpc.test.ts @@ -102,6 +102,24 @@ describe('substituteRemotables', () => { }); }); + it.each([ + ['a bare promise', (): unknown => new Promise(() => undefined)], + [ + 'a nested promise', + (): unknown => ({ inner: new Promise(() => undefined) }), + ], + ['a promise in an array', (): unknown => [new Promise(() => undefined)]], + ['a foreign thenable', (): unknown => ({ then: () => undefined })], + ])('refuses to serialize %s', (_label, make) => { + const assign = (): string => 'j1'; + // A promise has no own enumerable properties, so walking it would yield + // `{}` and JSON.stringify would accept that — the client would receive a + // success response with the value silently gone. + expect(() => substituteRemotables(make(), isFakeRemotable, assign)).toThrow( + /unsettled promise/u, + ); + }); + it('leaves primitives and non-remotable objects alone', () => { const assign = (): string => 'unused'; expect(substituteRemotables(42, isFakeRemotable, assign)).toBe(42); diff --git a/packages/ocap-jsonrpc-vat/src/json-rpc.ts b/packages/ocap-jsonrpc-vat/src/json-rpc.ts index dc3ea4631b..34384b2647 100644 --- a/packages/ocap-jsonrpc-vat/src/json-rpc.ts +++ b/packages/ocap-jsonrpc-vat/src/json-rpc.ts @@ -118,6 +118,23 @@ export function expandMarkers( return value; } +/** + * Identify a thenable. Checked structurally rather than via `passStyleOf` + * so this module stays free of environment assumptions — it takes + * `isRemotable` as a hook for the same reason — and so that a CapTP promise + * or any other foreign thenable is recognized alongside a native one. + * + * @param value - The value to test. + * @returns True if `value` has a callable `then`. + */ +function isThenable(value: unknown): boolean { + return ( + typeof value === 'object' && + value !== null && + typeof (value as { then?: unknown }).then === 'function' + ); +} + /** * Walk `value`, replacing every remotable (as identified by * `isRemotable`) with `"${MARKER_PREFIX}${assign(remotable)}"`. @@ -126,6 +143,9 @@ export function expandMarkers( * * The result is a JSON-safe tree ready for `JSON.stringify`. * + * @throws If the tree contains an unsettled promise, which would otherwise + * serialize to `{}` and reach the client as a silently wrong success. + * * @param value - The value to walk. * @param isRemotable - Predicate identifying a value that should be * substituted for a marker. @@ -144,6 +164,19 @@ export function substituteRemotables( if (Array.isArray(value)) { return value.map((item) => substituteRemotables(item, isRemotable, assign)); } + if (isThenable(value)) { + // A promise has no own enumerable properties, so the object walk below + // would quietly turn it into `{}` — and `JSON.stringify` would accept + // that, handing the client a plausible-looking success payload with the + // value silently missing. Refusing is the only honest option here: + // awaiting an arbitrarily nested promise could block the connection for + // as long as it stays unsettled. A method that returns a promise-valued + // field has to settle it before returning. + throw new BridgeRpcError( + JSON_RPC_ERROR.INTERNAL_ERROR, + 'result contains an unsettled promise, which has no JSON form', + ); + } if (typeof value === 'object' && value !== null) { const out: Record = {}; for (const [key, val] of Object.entries(value as Record)) { From 585818a2c004cb440ba62900cf53c3796e161e72 Mon Sep 17 00:00:00 2001 From: Chip Morningstar Date: Thu, 6 Aug 2026 14:39:09 -0700 Subject: [PATCH 19/24] docs(ocap-jsonrpc-vat): stale o in module docstring is now j --- packages/ocap-jsonrpc-vat/src/json-rpc.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ocap-jsonrpc-vat/src/json-rpc.ts b/packages/ocap-jsonrpc-vat/src/json-rpc.ts index 34384b2647..5cb317e659 100644 --- a/packages/ocap-jsonrpc-vat/src/json-rpc.ts +++ b/packages/ocap-jsonrpc-vat/src/json-rpc.ts @@ -4,7 +4,7 @@ * * Object references are named via the sigil convention `"@@NAME"` (NAME * one or more alphanumeric characters). The mediator assigns names of the - * form `o`; other allocation schemes remain compatible with the walker. + * form `j`; other allocation schemes remain compatible with the walker. */ /** Full sigil string prefix (two `@`). */ From 7747dce7a7306ba1dbdf7dd78ea8fe2c66079191 Mon Sep 17 00:00:00 2001 From: Chip Morningstar Date: Thu, 6 Aug 2026 15:01:34 -0700 Subject: [PATCH 20/24] fix(ocap-kernel): reject, don't throw, for an unregistered kernel service A throw escaped the crank and killed the run loop. The init sweep cannot prevent this: a (1,1) refcount baseline keeps the object alive. --- packages/ocap-kernel/CHANGELOG.md | 5 +- packages/ocap-kernel/src/Kernel.ts | 13 ++- .../src/KernelServiceManager.test.ts | 34 +++++++- .../ocap-kernel/src/KernelServiceManager.ts | 84 +++++++++++++------ packages/ocap-kernel/src/io/io-service.ts | 11 +-- 5 files changed, 109 insertions(+), 38 deletions(-) diff --git a/packages/ocap-kernel/CHANGELOG.md b/packages/ocap-kernel/CHANGELOG.md index 98961456dd..588c44df69 100644 --- a/packages/ocap-kernel/CHANGELOG.md +++ b/packages/ocap-kernel/CHANGELOG.md @@ -10,7 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - Add `IOListener`, an endpoint peers connect to that yields one `IOChannel` per connection via `accept()`, replacing the previous one-client-at-a-time channel. Each accepted connection is a distinct object, so holding one conveys no way to reach another, and `direction` is enforced per connection. `accept()` resolves `null` once the listener is closed so an accept loop can terminate rather than hang ([#1007](https://github.com/MetaMask/ocap-kernel/pull/1007)) -- Anonymous kernel-hosted objects are recorded persistently and swept at kernel init, so one abandoned by a previous incarnation is neither left pinned forever nor able to take down the run loop when something is delivered to it — an anonymous object has no name to be re-registered under on boot, unlike a named service ([#1007](https://github.com/MetaMask/ocap-kernel/pull/1007)) +- Anonymous kernel-hosted objects are recorded persistently and swept at kernel init, so one abandoned by a previous incarnation is not left pinned forever, accumulating with every restart — an anonymous object has no name to be re-registered under on boot, unlike a named service. The sweep unpins but cannot delete an object a vat import or queued message still references, so it does not by itself make a delivery to a survivor safe; the `invokeKernelService` fix below is what does ([#1007](https://github.com/MetaMask/ocap-kernel/pull/1007)) - Add `KernelServiceManager.registerAnonymousKernelObject()` / `releaseAnonymousKernelObject()`, which make a kernel-hosted object routable by kref without entering it in the service-name index, so it has no name in the global service namespace and cannot be requested via a cluster config's `services` list. Used to host accepted IO connections, whose authority comes from holding the reference ([#1007](https://github.com/MetaMask/ocap-kernel/pull/1007)) - Report run loop health in `KernelStatus.runLoop` (`{ state: 'idle' | 'running' }` or `{ state: 'failed', error, detail }`), exporting `RunLoopStatus`, `RunLoopStatusStruct`, and `OnRunLoopFailure` ([#1005](https://github.com/MetaMask/ocap-kernel/pull/1005)) - `idle` means never started; a loop parked on an empty queue reports `running` @@ -47,6 +47,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- A message delivered to a kernel-owned kref with no registered service now rejects the caller with `ENDPOINT_UNREACHABLE` instead of throwing, which escaped the crank and killed the run loop — turning one unreachable reference into a dead kernel ([#1007](https://github.com/MetaMask/ocap-kernel/pull/1007)) + - Reachable without any kernel bug: an anonymous kernel object hosts something that cannot outlive the process, such as an accepted socket connection, so a vat holding one across a restart or a message to one still queued from the previous incarnation lands here. Kernel objects are born with a `(1, 1)` refcount, so the init sweep cannot delete such an object and its `kernel` owner survives (see [#1006](https://github.com/MetaMask/ocap-kernel/issues/1006)) + - Matches what `KernelRouter` already does for a delivery whose endpoint has vanished. A message sent with no result promise has nobody to report to, so it is logged instead - The kernel run queue no longer strands messages, going quiet with no error, no log, and no crash. `runQueueLengthCache` uses a negative value to mean "unknown, re-read from the database", but `enqueueRun`/`dequeueRun` adjusted it arithmetically without materializing it first — so an enqueue while the cache held `-1` produced `0` for a queue that actually held an item, and because `0` is not negative it was never re-read again. The run loop then saw an empty queue, went to sleep, and stranded everything behind it. Two paths reach that `-1`: kernel startup, and `rollbackCrank`, which invalidates the cache because a rollback may have restored dequeued items. The rollback path is the more likely of the two in practice, since a rollback is normally followed immediately by enqueueing an error or termination message. The run loop is also now woken by any non-empty queue rather than only by the empty-to-one transition, so a drifted count cannot silently lose the wakeup either ([#1007](https://github.com/MetaMask/ocap-kernel/pull/1007)) - Stop reporting a healthy kernel after the run loop dies ([#1005](https://github.com/MetaMask/ocap-kernel/pull/1005)) - The error was logged and swallowed, so `getStatus` kept returning its healthy-looking record while nothing on the run queue was processed and every `queueMessage` hung forever. Results in flight now reject with the killing error as their `cause`, later calls reject immediately, and `getStatus` answers without waiting on a crank that may never end diff --git a/packages/ocap-kernel/src/Kernel.ts b/packages/ocap-kernel/src/Kernel.ts index f99bebfd45..a312e60e65 100644 --- a/packages/ocap-kernel/src/Kernel.ts +++ b/packages/ocap-kernel/src/Kernel.ts @@ -297,10 +297,15 @@ export class Kernel { // Discard anonymous kernel objects from a previous incarnation. They // host things that cannot outlive the process — an accepted socket // connection, say — and unlike a named service there is no name to - // re-register one under, so a survivor is unreachable but still pinned. - // Same hazard the facet registration above guards against: a delivery to - // one would find nothing registered and kill the run queue. Swept before - // the queue starts for exactly that reason. + // re-register one under, so a survivor is unreachable but still pinned, + // accumulating with every restart. + // + // This unpins; it does not by itself make a delivery to a survivor safe, + // because the object outlives the sweep whenever a vat import or queued + // message still references it. `invokeKernelService` is what makes that + // case survivable, by rejecting the caller instead of throwing. Swept + // before the queue starts regardless, so the unreachable objects are gone + // before anything can address them. const abandoned = this.#kernelServiceManager.releaseAbandonedAnonymousKernelObjects(); if (abandoned > 0) { diff --git a/packages/ocap-kernel/src/KernelServiceManager.test.ts b/packages/ocap-kernel/src/KernelServiceManager.test.ts index ffbffc9792..a3efd11bf9 100644 --- a/packages/ocap-kernel/src/KernelServiceManager.test.ts +++ b/packages/ocap-kernel/src/KernelServiceManager.test.ts @@ -386,14 +386,44 @@ describe('KernelServiceManager', () => { expect(mockKernelQueue.resolvePromises).not.toHaveBeenCalled(); }); - it('throws error for non-existent service', () => { + it('rejects the caller for a non-existent service', () => { const message: KernelMessage = { methargs: kser(['testMethod', []]), + result: 'kp1', }; + // Must not throw: a throw here escapes the crank and kills the run + // loop, and this is reachable whenever an anonymous kernel object + // outlives the process that hosted it. expect(() => serviceManager.invokeKernelService('ko999', message), - ).toThrow('No registered service for ko999'); + ).not.toThrow(); + expect(mockKernelQueue.resolvePromises).toHaveBeenCalledWith('kernel', [ + [ + 'kp1', + true, + makeKernelError( + 'ENDPOINT_UNREACHABLE', + 'No registered service for ko999', + ), + ], + ]); + }); + + it('logs for a non-existent service when the message has no result', () => { + const loggerErrorSpy = vi.spyOn(logger, 'error'); + const message: KernelMessage = { + methargs: kser(['testMethod', []]), + }; + + expect(() => + serviceManager.invokeKernelService('ko999', message), + ).not.toThrow(); + expect(loggerErrorSpy).toHaveBeenCalledWith( + 'Error in kernel service method:', + 'No registered service for ko999', + ); + expect(mockKernelQueue.resolvePromises).not.toHaveBeenCalled(); }); it('handles unknown method with result', async () => { diff --git a/packages/ocap-kernel/src/KernelServiceManager.ts b/packages/ocap-kernel/src/KernelServiceManager.ts index c0ad7262a9..bc236e6113 100644 --- a/packages/ocap-kernel/src/KernelServiceManager.ts +++ b/packages/ocap-kernel/src/KernelServiceManager.ts @@ -1,4 +1,5 @@ import { E } from '@endo/eventual-send'; +import type { ExpectedKernelErrorCode } from '@metamask/kernel-errors'; import type { Logger } from '@metamask/logger'; import type { KernelQueue } from './KernelQueue.ts'; @@ -157,14 +158,18 @@ export class KernelServiceManager { * Discard anonymous kernel objects left behind by a previous incarnation. * * These exist to host things that cannot outlive the process — an accepted - * socket connection, say — so any that survived a restart are garbage. They - * are also actively harmful if left: still pinned, so they accumulate with - * every restart, and still owned by `'kernel'`, so a delivery to one would - * reach `invokeKernelService`, find nothing registered, throw, and take the - * run loop down with it. + * socket connection, say — so any that survived a restart are garbage, and + * harmful if left: still pinned, so they accumulate with every restart. * - * Must run before the run queue starts, so nothing can be delivered to a - * stale kref in the window before the sweep. + * Note what this does *not* guarantee. The kernel object is deleted only + * once nothing references it, which with the current `(1, 1)` refcount + * baseline (see #1006) is never; a survivor therefore keeps its `'kernel'` + * owner, and a delivery to it still routes to `invokeKernelService`. That + * case is made survivable there, by rejecting the caller's promise rather + * than throwing, and not here. + * + * Runs before the run queue starts, so the unpinning is complete before + * anything can address one of these krefs. * * @returns The number of objects discarded. */ @@ -254,7 +259,25 @@ export class KernelServiceManager { invokeKernelService(target: KRef, message: KernelMessage): void { const kernelService = this.#kernelServicesByObject.get(target); if (!kernelService) { - throw Error(`No registered service for ${target}`); + // Reachable, and not necessarily a kernel bug: an anonymous kernel + // object hosts something that cannot outlive the process, such as an + // accepted socket connection. A vat holding one across a restart, or a + // message to one still sitting in the run queue from the previous + // incarnation, arrives here with nothing registered. + // + // Rejecting rather than throwing is the point. A throw escapes the + // crank and takes the run loop with it, so one unreachable reference + // becomes a dead kernel — and the sweep in `Kernel.#init` cannot + // prevent that on its own, since the object survives with its `kernel` + // owner intact whenever a vat import or queued message still + // references it. This mirrors what `KernelRouter` already does for a + // delivery whose endpoint has vanished. + this.#failMessage( + message.result, + 'ENDPOINT_UNREACHABLE', + `No registered service for ${target}`, + ); + return; } const { methargs, result } = message; const [method, args] = kunser(methargs) as [string, unknown[]]; @@ -282,27 +305,36 @@ export class KernelServiceManager { return undefined; }) .catch((problem: unknown) => { - if (result) { - const detail = - problem instanceof Error ? problem.message : String(problem); - this.#kernelQueue.resolvePromises('kernel', [ - [result, true, makeKernelError('DELIVERY_FAILED', detail)], - ]); - } else { - this.#logger?.error('Error in kernel service method:', problem); - } + this.#failMessage(result, 'DELIVERY_FAILED', problem); }); } catch (syncError) { // Handle synchronous errors thrown before returning a Promise - if (result) { - const detail = - syncError instanceof Error ? syncError.message : String(syncError); - this.#kernelQueue.resolvePromises('kernel', [ - [result, true, makeKernelError('DELIVERY_FAILED', detail)], - ]); - } else { - this.#logger?.error('Error in kernel service method:', syncError); - } + this.#failMessage(result, 'DELIVERY_FAILED', syncError); + } + } + + /** + * Report a failed kernel service message by rejecting the caller's result + * promise. A message sent with no result promise has nobody to report to, + * so the problem is logged instead. + * + * @param result - The kref of the message's result promise, if it has one. + * @param code - The kernel error code to report to the caller. + * @param problem - The error or description of what went wrong. + */ + #failMessage( + result: KRef | null | undefined, + code: ExpectedKernelErrorCode, + problem: unknown, + ): void { + if (result) { + const detail = + problem instanceof Error ? problem.message : String(problem); + this.#kernelQueue.resolvePromises('kernel', [ + [result, true, makeKernelError(code, detail)], + ]); + } else { + this.#logger?.error('Error in kernel service method:', problem); } } } diff --git a/packages/ocap-kernel/src/io/io-service.ts b/packages/ocap-kernel/src/io/io-service.ts index 09fe79374a..9fb37b4931 100644 --- a/packages/ocap-kernel/src/io/io-service.ts +++ b/packages/ocap-kernel/src/io/io-service.ts @@ -27,11 +27,12 @@ export type ConnectionHost = { * A peer disconnecting ends the underlying transport and makes `read()` * report EOF, but does *not* by itself stop the kernel hosting this * object, because the holder still has a live reference to it. Releasing - * on EOF instead would be actively worse than leaking: the vat's c-list - * still names the kref, so a subsequent call on the dropped reference - * would route to `invokeKernelService`, find nothing registered, and - * throw — which takes down the run loop. Until a vat dropping the - * reference is itself observable (see #1006), unreleased connections are + * on EOF instead would be worse than leaking: the vat's c-list still names + * the kref, so a subsequent call on the dropped reference would route to + * `invokeKernelService` and find nothing registered, failing the caller + * with `ENDPOINT_UNREACHABLE` for a connection it never released. Until a + * vat dropping the reference is itself observable (see #1006), unreleased + * connections are * bounded by their listener's lifetime: `close()` on the listener * releases whatever it handed out, and `IOManager` releases the rest when * the subcluster goes away. From f7570dffc0d872e4fdce516d66dbccfabde5d438 Mon Sep 17 00:00:00 2001 From: Chip Morningstar Date: Thu, 6 Aug 2026 18:58:03 -0700 Subject: [PATCH 21/24] chore: retrigger CI From da0133ec60ce3e24257a6553bbb64b6ec212a85f Mon Sep 17 00:00:00 2001 From: Chip Morningstar Date: Fri, 7 Aug 2026 13:49:50 -0700 Subject: [PATCH 22/24] fix(ocap-jsonrpc-vat): refuse non-finite numbers in results MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit JSON.stringify turns NaN and ±Infinity into null, which is indistinguishable from the null a void method returns. --- packages/ocap-jsonrpc-vat/CHANGELOG.md | 2 +- .../ocap-jsonrpc-vat/src/json-rpc.test.ts | 25 +++++++++++++++++++ packages/ocap-jsonrpc-vat/src/json-rpc.ts | 22 ++++++++++++++-- 3 files changed, 46 insertions(+), 3 deletions(-) diff --git a/packages/ocap-jsonrpc-vat/CHANGELOG.md b/packages/ocap-jsonrpc-vat/CHANGELOG.md index 04049d6f9f..ed70dc8ffa 100644 --- a/packages/ocap-jsonrpc-vat/CHANGELOG.md +++ b/packages/ocap-jsonrpc-vat/CHANGELOG.md @@ -12,7 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Initial release: vat serving a line-delimited JSON-RPC 2.0 protocol on a Unix-domain-socket `IOService` endowment - `redeemURL(url)` redeems an OCAP URL through the kernel's `ocapURLRedemptionService` and returns a sigil name of the form `@@j` referring to the resulting live reference - `send(target, method, args)` invokes `E(target)[method](...args)` with `@@j` markers in `args` expanded to their live references and any remotable in the result substituted for its sigil name - - A result containing an unsettled promise is refused with an internal error rather than serialized: a promise has no own enumerable properties, so it would otherwise become `{}` in a response that `JSON.stringify` accepts, silently handing the client a success payload with the value missing + - A result is refused with an internal error, rather than serialized, when it holds a value that `JSON.stringify` accepts but cannot represent — an unsettled promise, which has no own enumerable properties and would become `{}`, or a non-finite number (`NaN`, `±Infinity`), which would become `null`. Either would otherwise hand the client a success payload whose value is silently wrong, and `null` in particular is indistinguishable from the `null` a void method legitimately returns. `-0` is allowed through, since it serializes to a numerically equal `0` - Session state is in-memory only and resets on socket disconnect [Unreleased]: https://github.com/MetaMask/ocap-kernel/ diff --git a/packages/ocap-jsonrpc-vat/src/json-rpc.test.ts b/packages/ocap-jsonrpc-vat/src/json-rpc.test.ts index fd126c2973..83dff2bea6 100644 --- a/packages/ocap-jsonrpc-vat/src/json-rpc.test.ts +++ b/packages/ocap-jsonrpc-vat/src/json-rpc.test.ts @@ -120,6 +120,31 @@ describe('substituteRemotables', () => { ); }); + it.each([ + ['NaN', (): unknown => Number.NaN, /NaN/u], + ['Infinity', (): unknown => Number.POSITIVE_INFINITY, /Infinity/u], + ['-Infinity', (): unknown => Number.NEGATIVE_INFINITY, /-Infinity/u], + ['a nested NaN', (): unknown => ({ ratio: Number.NaN }), /NaN/u], + [ + 'an Infinity in an array', + (): unknown => [Number.POSITIVE_INFINITY], + /Infinity/u, + ], + ])('rejects %s rather than emitting null', (_label, make, pattern) => { + const assign = (): string => 'j1'; + // JSON.stringify turns a non-finite number into `null`, which is exactly + // what a void method produces — so the client cannot tell a missing value + // from a real one. + expect(() => substituteRemotables(make(), isFakeRemotable, assign)).toThrow( + pattern, + ); + }); + + it('leaves -0 alone, since it serializes to a numerically equal 0', () => { + const assign = (): string => 'unused'; + expect(substituteRemotables(-0, isFakeRemotable, assign)).toBe(-0); + }); + it('leaves primitives and non-remotable objects alone', () => { const assign = (): string => 'unused'; expect(substituteRemotables(42, isFakeRemotable, assign)).toBe(42); diff --git a/packages/ocap-jsonrpc-vat/src/json-rpc.ts b/packages/ocap-jsonrpc-vat/src/json-rpc.ts index 5cb317e659..d909d4410d 100644 --- a/packages/ocap-jsonrpc-vat/src/json-rpc.ts +++ b/packages/ocap-jsonrpc-vat/src/json-rpc.ts @@ -143,8 +143,10 @@ function isThenable(value: unknown): boolean { * * The result is a JSON-safe tree ready for `JSON.stringify`. * - * @throws If the tree contains an unsettled promise, which would otherwise - * serialize to `{}` and reach the client as a silently wrong success. + * @throws If the tree contains a value with no JSON form that + * `JSON.stringify` would nonetheless accept — an unsettled promise (which + * becomes `{}`) or a non-finite number (which becomes `null`) — since either + * would reach the client as a silently wrong success. * * @param value - The value to walk. * @param isRemotable - Predicate identifying a value that should be @@ -177,6 +179,22 @@ export function substituteRemotables( 'result contains an unsettled promise, which has no JSON form', ); } + if (typeof value === 'number' && !Number.isFinite(value)) { + // JSON has no way to write `NaN` or `±Infinity`, and `JSON.stringify` + // does not complain — it emits `null`. That is indistinguishable from + // the `null` a void method legitimately produces (`successResponse` + // normalizes `undefined` to `null`), so the client cannot tell a missing + // value from a real one. Same reasoning as the promise case above: + // silently wrong is worse than an explicit failure. + // + // `-0` is deliberately allowed through. It serializes to `0`, which is + // a numerically equal JSON number rather than a value replaced by an + // unrelated one. + throw new BridgeRpcError( + JSON_RPC_ERROR.INTERNAL_ERROR, + `result contains ${String(value)}, which has no JSON form`, + ); + } if (typeof value === 'object' && value !== null) { const out: Record = {}; for (const [key, val] of Object.entries(value as Record)) { From 746450a3590d5629ba00efeb8b373234850e8ca7 Mon Sep 17 00:00:00 2001 From: Chip Morningstar Date: Fri, 7 Aug 2026 14:15:27 -0700 Subject: [PATCH 23/24] fix(ocap-jsonrpc-vat): commit reference names only for a sendable reply A request that failed partway left its @@j names in the table, and sequential names make an undisclosed one guessable. --- packages/ocap-jsonrpc-vat/CHANGELOG.md | 1 + packages/ocap-jsonrpc-vat/src/bridge.test.ts | 157 +++++++++++++++++++ packages/ocap-jsonrpc-vat/src/bridge.ts | 67 +++++++- packages/ocap-jsonrpc-vat/src/vat/index.ts | 12 +- 4 files changed, 231 insertions(+), 6 deletions(-) diff --git a/packages/ocap-jsonrpc-vat/CHANGELOG.md b/packages/ocap-jsonrpc-vat/CHANGELOG.md index ed70dc8ffa..896f28ebe2 100644 --- a/packages/ocap-jsonrpc-vat/CHANGELOG.md +++ b/packages/ocap-jsonrpc-vat/CHANGELOG.md @@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `redeemURL(url)` redeems an OCAP URL through the kernel's `ocapURLRedemptionService` and returns a sigil name of the form `@@j` referring to the resulting live reference - `send(target, method, args)` invokes `E(target)[method](...args)` with `@@j` markers in `args` expanded to their live references and any remotable in the result substituted for its sigil name - A result is refused with an internal error, rather than serialized, when it holds a value that `JSON.stringify` accepts but cannot represent — an unsettled promise, which has no own enumerable properties and would become `{}`, or a non-finite number (`NaN`, `±Infinity`), which would become `null`. Either would otherwise hand the client a success payload whose value is silently wrong, and `null` in particular is indistinguishable from the `null` a void method legitimately returns. `-0` is allowed through, since it serializes to a numerically equal `0` + - Name disclosure is atomic per request: the `@@j` names minted while walking a result are committed only once the reply is known to be a sendable success, and discarded otherwise. Without this, a request that failed partway — or whose reply could not be encoded — left its names in the connection's table, and since names are sequential the client could reach those references by guessing, having been told only that the call failed. Encodability is therefore settled in the bridge, where the names are, rather than in the writer - Session state is in-memory only and resets on socket disconnect [Unreleased]: https://github.com/MetaMask/ocap-kernel/ diff --git a/packages/ocap-jsonrpc-vat/src/bridge.test.ts b/packages/ocap-jsonrpc-vat/src/bridge.test.ts index b852fc54eb..463b01ff49 100644 --- a/packages/ocap-jsonrpc-vat/src/bridge.test.ts +++ b/packages/ocap-jsonrpc-vat/src/bridge.test.ts @@ -512,3 +512,160 @@ describe('resetSession', () => { expect(second.result).toBe('@@j1'); }); }); + +describe('dispatch: name disclosure is atomic', () => { + const redeemFake = async (): Promise => makeFake('root'); + + /** + * Redeem a URL so the connection has a usable target, returning its name. + * + * @param bridge - The bridge to prime. + * @returns The marker string naming the redeemed object. + */ + async function primeTarget( + bridge: ReturnType, + ): Promise { + const response = (await bridge.dispatch({ + jsonrpc: '2.0', + id: 'prime', + method: 'redeemURL', + params: { url: 'ocap://root' }, + })) as { result: string }; + return response.result; + } + + it.each([ + [ + 'a non-finite number', + (): unknown => ({ ref: makeFake('leaked'), bad: Number.NaN }), + ], + [ + 'an unsettled promise', + (): unknown => ({ + ref: makeFake('leaked'), + bad: new Promise(() => undefined), + }), + ], + ['a bigint', (): unknown => ({ ref: makeFake('leaked'), bad: 1n })], + ])( + 'discards names minted for a reply rejected over %s', + async (_label, makeResult) => { + const { bridge } = buildBridge({ + redeem: redeemFake, + invoke: async (): Promise => makeResult(), + }); + const target = await primeTarget(bridge); + + const failed = (await bridge.dispatch({ + jsonrpc: '2.0', + id: 1, + method: 'send', + params: { target, method: 'getRef', args: [] }, + })) as { error?: { code: number } }; + expect(failed.error).toBeDefined(); + + // The walk minted a name for `ref` before hitting the bad value. Names + // are sequential, so guessing it takes no work — it must not resolve. + const probe = (await bridge.dispatch({ + jsonrpc: '2.0', + id: 2, + method: 'send', + params: { target: '@@j2', method: 'anything', args: [] }, + })) as { error?: { code: number; message: string } }; + expect(probe.error?.code).toBe(JSON_RPC_ERROR.INVALID_PARAMS); + expect(probe.error?.message).toMatch(/not a known reference/u); + }, + ); + + it('keeps a name already disclosed by an earlier successful reply', async () => { + const shared = makeFake('shared'); + let failNext = false; + const { bridge } = buildBridge({ + redeem: redeemFake, + invoke: async (): Promise => + failNext ? { ref: shared, bad: Number.NaN } : shared, + }); + const target = await primeTarget(bridge); + + const first = (await bridge.dispatch({ + jsonrpc: '2.0', + id: 1, + method: 'send', + params: { target, method: 'getShared', args: [] }, + })) as { result: string }; + const disclosed = first.result; + + // A later failed request mentions the same object. Rolling that request + // back must not revoke a name the client was legitimately given. + failNext = true; + const failed = (await bridge.dispatch({ + jsonrpc: '2.0', + id: 2, + method: 'send', + params: { target, method: 'getShared', args: [] }, + })) as { error?: unknown }; + expect(failed.error).toBeDefined(); + + failNext = false; + const after = (await bridge.dispatch({ + jsonrpc: '2.0', + id: 3, + method: 'send', + params: { target: disclosed, method: 'stillThere', args: [] }, + })) as { result?: unknown; error?: unknown }; + expect(after.error).toBeUndefined(); + }); + + it('reuses the id a rolled-back name held, leaving no gap', async () => { + let failNext = true; + const { bridge } = buildBridge({ + redeem: redeemFake, + invoke: async (): Promise => + failNext + ? { ref: makeFake('discarded'), bad: Number.NaN } + : makeFake('kept'), + }); + const target = await primeTarget(bridge); + + await bridge.dispatch({ + jsonrpc: '2.0', + id: 1, + method: 'send', + params: { target, method: 'fails', args: [] }, + }); + failNext = false; + const ok = (await bridge.dispatch({ + jsonrpc: '2.0', + id: 2, + method: 'send', + params: { target, method: 'works', args: [] }, + })) as { result: string }; + + // The discarded name was never disclosed, so its id is free to reuse. + expect(ok.result).toBe('@@j2'); + }); + + it('still registers names for a reply that succeeds', async () => { + const { bridge } = buildBridge({ + redeem: redeemFake, + invoke: async (): Promise => makeFake('handed over'), + }); + const target = await primeTarget(bridge); + + const response = (await bridge.dispatch({ + jsonrpc: '2.0', + id: 1, + method: 'send', + params: { target, method: 'getRef', args: [] }, + })) as { result: string }; + expect(response.result).toBe('@@j2'); + + const reuse = (await bridge.dispatch({ + jsonrpc: '2.0', + id: 2, + method: 'send', + params: { target: response.result, method: 'usable', args: [] }, + })) as { error?: unknown }; + expect(reuse.error).toBeUndefined(); + }); +}); diff --git a/packages/ocap-jsonrpc-vat/src/bridge.ts b/packages/ocap-jsonrpc-vat/src/bridge.ts index 8793ed3ee8..d52fd0c081 100644 --- a/packages/ocap-jsonrpc-vat/src/bridge.ts +++ b/packages/ocap-jsonrpc-vat/src/bridge.ts @@ -79,24 +79,57 @@ export function makeBridge(hooks: BridgeHooks): Bridge { */ const where = (): string => hooks.label ?? 'this connection'; + /** + * Names minted while handling the current request and not yet disclosed to + * the client. A name only becomes usable once the client has actually been + * sent a reply carrying it; see `dispatch`. + */ + let stagedNames: string[] = []; + + /** `nextObjId` as of the start of the current request, for rollback. */ + let objIdBeforeRequest = 0; + const resetSession = (): void => { nameToObj = new Map(); objToName = new Map(); nextObjId = 0; + stagedNames = []; + objIdBeforeRequest = 0; }; const assignName = (obj: unknown): string => { const existing = objToName.get(obj); if (existing !== undefined) { + // Already disclosed by an earlier reply, so it is not this request's to + // stage — and must survive if this request is rolled back. return existing; } nextObjId += 1; const name = `j${nextObjId}`; nameToObj.set(name, obj); objToName.set(obj, name); + stagedNames.push(name); return name; }; + /** + * Discard the names minted for the current request, so a reply the client + * never received leaves no reachable reference behind. + * + * `nextObjId` is rewound too. Reusing an id is safe precisely because a + * rolled-back name was never disclosed: no client can be holding it. + */ + const rollbackNames = (): void => { + for (const name of stagedNames) { + if (nameToObj.has(name)) { + objToName.delete(nameToObj.get(name)); + nameToObj.delete(name); + } + } + nextObjId = objIdBeforeRequest; + stagedNames = []; + }; + const resolveName = (name: string): unknown => nameToObj.get(name); const handleRedeemURL = async (params: unknown): Promise => { @@ -126,7 +159,7 @@ export function makeBridge(hooks: BridgeHooks): Bridge { return substituteRemotables(result, hooks.isRemotable, assignName); }; - const dispatch = async (request: unknown): Promise => { + const handleRequest = async (request: unknown): Promise => { const id = extractId(request); if (!isJsonRpcRequest(request)) { return errorResponse( @@ -164,6 +197,38 @@ export function makeBridge(hooks: BridgeHooks): Bridge { } }; + const dispatch = async (request: unknown): Promise => { + objIdBeforeRequest = nextObjId; + stagedNames = []; + const response = await handleRequest(request); + if ('error' in response) { + // The client is being told the call failed, so it must not come away + // able to reach references the result walk minted before giving up. + // Names are sequential, so an undisclosed one is trivially guessable. + rollbackNames(); + return response; + } + try { + // A name becomes reachable only for a reply that can actually be sent. + // `JSON.stringify` still throws on values the walkers do not screen — + // a bigint, or a circular structure — and that failure has to roll the + // names back as well, which is why encodability is settled here rather + // than left to whoever writes the reply. Costs one extra encode per + // request, which is worth it to keep the two decisions in one place. + JSON.stringify(response); + } catch (error) { + rollbackNames(); + return errorResponse( + response.id, + JSON_RPC_ERROR.INTERNAL_ERROR, + 'result could not be encoded as JSON', + error instanceof Error ? error.message : String(error), + ); + } + stagedNames = []; + return response; + }; + return { dispatch, resetSession }; } diff --git a/packages/ocap-jsonrpc-vat/src/vat/index.ts b/packages/ocap-jsonrpc-vat/src/vat/index.ts index 5bc2a65efc..3129bcda84 100644 --- a/packages/ocap-jsonrpc-vat/src/vat/index.ts +++ b/packages/ocap-jsonrpc-vat/src/vat/index.ts @@ -143,11 +143,13 @@ export function buildRootObject( /** * Encode and write one response. * - * Encoding can fail even when dispatch succeeded, because a method may - * return a passable that has no JSON form — a `bigint`, say — which - * `substituteRemotables` passes through untouched. Sending an error in - * that case keeps the exchange to one reply per request; treating it as a - * write failure would drop the connection and leave the client waiting. + * The encode guard here is now defensive rather than load-bearing: a + * response from `dispatch` has already been proven encodable, because the + * bridge has to know whether the reply is sendable before it commits the + * `@@j` names minted for it. This still covers the responses built + * directly in this module, and keeps a `JSON.stringify` throw from being + * reported as a write failure, which would drop the connection and leave + * the client waiting instead of answering it. * * @param connection - The connection to write to. * @param response - The response to encode and send. From d0a600b7ee83723d538b8e6a25ff00264a64d1fb Mon Sep 17 00:00:00 2001 From: Chip Morningstar Date: Mon, 10 Aug 2026 11:47:50 -0700 Subject: [PATCH 24/24] test(ocap-jsonrpc-vat): cover per-connection name tables and accept liveness Also reuse MARKER_PATTERN/MARKER_PREFIX instead of literals, and drop the demo-specific VPS rehearsal doc, which belongs on the demo branch. --- packages/ocap-jsonrpc-vat/CHANGELOG.md | 1 + .../ocap-jsonrpc-vat/scripts/VPS-REHEARSAL.md | 87 ---------- packages/ocap-jsonrpc-vat/src/bridge.ts | 5 +- .../ocap-jsonrpc-vat/src/vat/index.test.ts | 155 ++++++++++++++++++ packages/ocap-jsonrpc-vat/test/helpers.ts | 124 ++++++++++++++ packages/ocap-jsonrpc-vat/tsconfig.json | 1 + 6 files changed, 284 insertions(+), 89 deletions(-) delete mode 100644 packages/ocap-jsonrpc-vat/scripts/VPS-REHEARSAL.md create mode 100644 packages/ocap-jsonrpc-vat/src/vat/index.test.ts create mode 100644 packages/ocap-jsonrpc-vat/test/helpers.ts diff --git a/packages/ocap-jsonrpc-vat/CHANGELOG.md b/packages/ocap-jsonrpc-vat/CHANGELOG.md index 896f28ebe2..92770c9094 100644 --- a/packages/ocap-jsonrpc-vat/CHANGELOG.md +++ b/packages/ocap-jsonrpc-vat/CHANGELOG.md @@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `send(target, method, args)` invokes `E(target)[method](...args)` with `@@j` markers in `args` expanded to their live references and any remotable in the result substituted for its sigil name - A result is refused with an internal error, rather than serialized, when it holds a value that `JSON.stringify` accepts but cannot represent — an unsettled promise, which has no own enumerable properties and would become `{}`, or a non-finite number (`NaN`, `±Infinity`), which would become `null`. Either would otherwise hand the client a success payload whose value is silently wrong, and `null` in particular is indistinguishable from the `null` a void method legitimately returns. `-0` is allowed through, since it serializes to a numerically equal `0` - Name disclosure is atomic per request: the `@@j` names minted while walking a result are committed only once the reply is known to be a sendable success, and discarded otherwise. Without this, a request that failed partway — or whose reply could not be encoded — left its names in the connection's table, and since names are sequential the client could reach those references by guessing, having been told only that the call failed. Encodability is therefore settled in the bridge, where the names are, rather than in the writer + - Each connection is served concurrently with its own bridge, and so its own `@@j` table: a name minted on one connection does not resolve on another, both connections mint names from their own counter, and a peer that stalls without hanging up does not stop new peers being accepted - Session state is in-memory only and resets on socket disconnect [Unreleased]: https://github.com/MetaMask/ocap-kernel/ diff --git a/packages/ocap-jsonrpc-vat/scripts/VPS-REHEARSAL.md b/packages/ocap-jsonrpc-vat/scripts/VPS-REHEARSAL.md deleted file mode 100644 index 9e684eab21..0000000000 --- a/packages/ocap-jsonrpc-vat/scripts/VPS-REHEARSAL.md +++ /dev/null @@ -1,87 +0,0 @@ -# VPS-side rehearsal notes for the ocap-jsonrpc-vat - -The vat replaces the openclaw plugins' shell-execed `ocap daemon -queueMessage`/`redeem-url` calls with a persistent JSON-RPC 2.0 -connection over a Unix socket. On the VPS it lives in the consumer -daemon (`~/.ocap-consumer`), which had no vats previously. - -Both routine restarts and cold resets are automated: - -- `rehearsal-restart-matcher.sh` — routine pre-rehearsal reset (URL, - registry, and vats stay put). Now includes a step 2b that runs - `start-ocap-jsonrpc-vat.sh --home ~/.ocap-consumer`. -- `reset-everything.sh` — cold reset with fresh URLs. Now includes a - step 7b that launches a fresh vat subcluster in the consumer - daemon. - -So the operator does not have to invoke the vat launcher directly in -normal rehearsal flow. The manual launcher (below) is only for -debugging or ad-hoc use. - -## Prerequisites - -- The chip/orchestration-demo branch is checked out at the same path - as before (openclaw plugins install with `-l` from the workspace, - so the branch update is picked up automatically). -- `yarn workspace @metamask/kernel-cli build` and - `yarn workspace @ocap/ocap-jsonrpc-vat build` have run at least - once since the branch update. - -## Manual launch (for debugging) - -```bash -./packages/ocap-jsonrpc-vat/scripts/start-ocap-jsonrpc-vat.sh \ - --home ~/.ocap-consumer -``` - -Confirm the socket: - -```bash -ls -l ~/.ocap-consumer/ocap-jsonrpc.sock -node ./packages/ocap-jsonrpc-vat/scripts/probe.mjs \ - ~/.ocap-consumer/ocap-jsonrpc.sock ocap:some@peer -``` - -The probe should print a `redeemURL` request whose response is either -a `@@j` marker (on success) or a `[KERNEL:DELIVERY_FAILED]` error -if the URL doesn't resolve or remote comms are down. - -## Openclaw plugin config - -For each of the three plugins in `~/.openclaw/openclaw.json` under -`plugins.entries` (`discovery`, `metamask`, `demo`): - -- **Remove** `ocapCliPath`. The plugin's config schema no longer - accepts it — leaving it in will fail plugin registration. -- **Add or change** `ocapHome` to `~/.ocap-consumer`. All three - plugins point at the same consumer-daemon socket. - -Alternatively set `socketPath` explicitly per plugin. - -Example diff: - -```jsonc -"discovery": { - "config": { -- "ocapCliPath": "/root/…/packages/kernel-cli/dist/app.mjs", -+ "ocapHome": "/root/.ocap-consumer", - "matcherUrl": "ocap:…" - } -} -``` - -Then restart openclaw (rehearsal-restart-matcher.sh does this in -step 3). - -## Sanity check before an LLM turn - -- `discovery_list_tracked` should show the matcher URL pre-redeemed - and its ref shown as `@@j` (was previously a kref). -- A `discovery_find_services` turn against the matcher should behave - as before — matcher on VPS, provider vats on laptop are untouched. - -## What changes on the laptop side - -Nothing structural. The laptop's provider vats and consumer daemon -keep their existing OCAP URLs. The plugins on VPS reach them through -the same libp2p path; only the plugin-to-kernel hop changed. diff --git a/packages/ocap-jsonrpc-vat/src/bridge.ts b/packages/ocap-jsonrpc-vat/src/bridge.ts index d52fd0c081..2963ba2c6d 100644 --- a/packages/ocap-jsonrpc-vat/src/bridge.ts +++ b/packages/ocap-jsonrpc-vat/src/bridge.ts @@ -11,6 +11,7 @@ import type { JsonRpcId, JsonRpcRequest, JsonRpcResponse } from './json-rpc.ts'; import { JSON_RPC_ERROR, + MARKER_PATTERN, MARKER_PREFIX, BridgeRpcError, expandMarkers, @@ -282,11 +283,11 @@ function requireSendParams(params: unknown): { 'params.target must be a string', ); } - const match = /^@@([A-Za-z0-9]+)$/u.exec(bag.target); + const match = MARKER_PATTERN.exec(bag.target); if (!match) { throw new BridgeRpcError( JSON_RPC_ERROR.INVALID_PARAMS, - 'params.target must be a marker string like "@@j1"', + `params.target must be a marker string like "${MARKER_PREFIX}j1"`, ); } if (typeof bag.method !== 'string') { diff --git a/packages/ocap-jsonrpc-vat/src/vat/index.test.ts b/packages/ocap-jsonrpc-vat/src/vat/index.test.ts new file mode 100644 index 0000000000..f1d9f7d9e4 --- /dev/null +++ b/packages/ocap-jsonrpc-vat/src/vat/index.test.ts @@ -0,0 +1,155 @@ +import type { Baggage } from '@metamask/ocap-kernel'; +import { describe, expect, it, vi } from 'vitest'; + +import { + makeMockBaggage, + makeMockConnection, + makeMockListener, + requestLine, +} from '../../test/helpers.ts'; +import { JSON_RPC_ERROR } from '../json-rpc.ts'; + +// The exo wrapper is irrelevant here and would need lockdown; the method bag +// is what these tests drive. Same approach as the other vat tests in the repo. +vi.mock('@metamask/kernel-utils/exo', () => ({ + makeDefaultExo: (_name: string, methods: Record) => methods, +})); + +// `E()` is not functional under `mock-endoify` — `HandledPromise` is absent, +// so any `E(x).m()` throws. Every target the vat reaches here is a local +// plain object, so identity is the faithful substitute, and what these tests +// cover is the shape of the accept/serve loops rather than eventual-send +// semantics. `bridge.ts` takes `redeem`/`invoke` as hooks for the same +// reason: it is meant to be exercised without a live kernel. +vi.mock('@endo/eventual-send', () => ({ + E: (target: unknown) => target, +})); + +const { buildRootObject } = await import('./index.ts'); + +type VatRoot = { + bootstrap: (vats: unknown, services: unknown) => Promise; +}; + +/** + * A stand-in for a reference a URL redeems to. Distinct per URL so a test + * can tell whose reference it is holding. + * + * @param label - Identifies which redemption produced this reference. + * @returns A callable stand-in reference. + */ +function makeRedeemed(label: string): { whoami: () => string } { + return { whoami: () => label }; +} + +/** + * Start the vat against a set of connections. + * + * @param connections - Connections for the listener to hand out. + * @returns The listener handle, so tests can count `accept()` calls. + */ +async function startVat( + connections: ReturnType[], +): Promise> { + const listener = makeMockListener(connections); + const root = buildRootObject( + undefined, + undefined, + makeMockBaggage() as unknown as Baggage, + ) as VatRoot; + await root.bootstrap( + {}, + { + ocapURLRedemptionService: { + redeem: async (url: string) => makeRedeemed(url), + }, + socket: listener.socket, + }, + ); + return listener; +} + +describe('accept loop: per-connection name tables', () => { + it('does not resolve a name minted on another connection', async () => { + const first = makeMockConnection([ + requestLine(1, 'redeemURL', { url: 'ocap://alpha' }), + ]); + // Forges the name the other connection was just given. + const second = makeMockConnection([ + requestLine(1, 'send', { target: '@@j1', method: 'whoami', args: [] }), + ]); + await startVat([first, second]); + + await vi.waitFor(() => { + expect(first.written).toHaveLength(1); + expect(second.written).toHaveLength(1); + }); + + expect(first.replies()[0]?.result).toBe('@@j1'); + const failure = second.replies()[0]?.error as { + code: number; + message: string; + }; + expect(failure.code).toBe(JSON_RPC_ERROR.INVALID_PARAMS); + expect(failure.message).toMatch(/not a known reference/u); + // The label makes it obvious which connection failed to resolve it. + expect(failure.message).toMatch(/connection 2/u); + }); + + it('mints the same name for different references on each connection', async () => { + const first = makeMockConnection([ + requestLine(1, 'redeemURL', { url: 'ocap://alpha' }), + requestLine(2, 'send', { target: '@@j1', method: 'whoami', args: [] }), + ]); + const second = makeMockConnection([ + requestLine(1, 'redeemURL', { url: 'ocap://beta' }), + requestLine(2, 'send', { target: '@@j1', method: 'whoami', args: [] }), + ]); + await startVat([first, second]); + + await vi.waitFor(() => { + expect(first.written).toHaveLength(2); + expect(second.written).toHaveLength(2); + }); + + // Both connections independently mint `@@j1` — the counters are their + // own — and each name resolves to that connection's own reference. + expect(first.replies()[0]?.result).toBe('@@j1'); + expect(second.replies()[0]?.result).toBe('@@j1'); + expect(first.replies()[1]?.result).toBe('ocap://alpha'); + expect(second.replies()[1]?.result).toBe('ocap://beta'); + }); +}); + +describe('accept loop: liveness', () => { + it('keeps serving new peers while an earlier one is stalled', async () => { + // Never sends anything and never hangs up. + const stalled = makeMockConnection([], { stall: true }); + const healthy = makeMockConnection([ + requestLine(1, 'redeemURL', { url: 'ocap://later' }), + ]); + const listener = await startVat([stalled, healthy]); + + await vi.waitFor(() => { + expect(healthy.written).toHaveLength(1); + }); + expect(healthy.replies()[0]?.result).toBe('@@j1'); + // Third accept() is the one that drains the queue and ends the loop, + // which can only happen if serving never blocked accepting. + await vi.waitFor(() => { + expect(listener.acceptCount()).toBe(3); + }); + expect(stalled.written).toHaveLength(0); + }); + + it('closes a connection once its peer goes away', async () => { + const connection = makeMockConnection([ + requestLine(1, 'redeemURL', { url: 'ocap://transient' }), + ]); + await startVat([connection]); + + await vi.waitFor(() => { + expect(connection.isClosed()).toBe(true); + }); + }); +}); diff --git a/packages/ocap-jsonrpc-vat/test/helpers.ts b/packages/ocap-jsonrpc-vat/test/helpers.ts new file mode 100644 index 0000000000..45e6e4512c --- /dev/null +++ b/packages/ocap-jsonrpc-vat/test/helpers.ts @@ -0,0 +1,124 @@ +/** + * Test helpers for driving the vat's accept loop against stand-in + * endowments. The vat only ever sees `accept()`, `read()`, `write()`, and + * `close()`, so a plain-object listener is enough to exercise the real + * wiring without a kernel. + */ + +/** + * Create a mock baggage store. + * + * @returns A mock baggage with Map semantics plus an `init` method. + */ +export function makeMockBaggage(): Map & { + init: (key: string, value: unknown) => void; +} { + const store = new Map(); + return Object.assign(store, { + init(key: string, value: unknown) { + if (store.has(key)) { + throw new Error(`Key already exists: ${key}`); + } + store.set(key, value); + }, + }); +} + +export type MockConnection = { + /** The connection as the vat sees it. */ + connection: { + read: () => Promise; + write: (data: string) => Promise; + close: () => Promise; + }; + /** Every line the vat has written, in order. */ + written: string[]; + /** Parsed view of `written`, for assertions. */ + replies: () => Record[]; + /** Whether the vat has closed this connection. */ + isClosed: () => boolean; +}; + +/** + * Create a mock connection that serves `lines` and then reports EOF. + * + * @param lines - Request lines to hand to the vat, in order. + * @param options - Behavior options. + * @param options.stall - When true, `read()` never settles once `lines` is + * drained, standing in for a peer that has gone quiet without hanging up. + * A stalled connection is what proves serving does not block accepting. + * @returns The connection plus inspection hooks. + */ +export function makeMockConnection( + lines: string[], + { stall = false }: { stall?: boolean } = {}, +): MockConnection { + const pending = [...lines]; + const written: string[] = []; + let closed = false; + return { + written, + replies: () => + written.map((line) => JSON.parse(line) as Record), + isClosed: () => closed, + connection: { + read: async (): Promise => { + const next = pending.shift(); + if (next !== undefined) { + return next; + } + if (stall) { + return new Promise(() => undefined); + } + return null; + }, + write: async (data: string): Promise => { + written.push(data); + }, + close: async (): Promise => { + closed = true; + }, + }, + }; +} + +/** + * Create a mock `IOListener` that hands out `connections` in order and then + * reports closure by resolving `null`. + * + * @param connections - The connections to yield from `accept()`. + * @returns The listener plus a count of `accept()` calls. + */ +export function makeMockListener(connections: MockConnection[]): { + socket: { accept: () => Promise }; + acceptCount: () => number; +} { + const queue = [...connections]; + let accepts = 0; + return { + acceptCount: () => accepts, + socket: { + accept: async (): Promise => { + accepts += 1; + const next = queue.shift(); + return next ? next.connection : null; + }, + }, + }; +} + +/** + * Build a JSON-RPC request line. + * + * @param id - The request id. + * @param method - The method to call. + * @param params - The params bag. + * @returns The encoded request line. + */ +export function requestLine( + id: number | string, + method: string, + params: unknown, +): string { + return JSON.stringify({ jsonrpc: '2.0', id, method, params }); +} diff --git a/packages/ocap-jsonrpc-vat/tsconfig.json b/packages/ocap-jsonrpc-vat/tsconfig.json index 7d727eadfe..4b626ea519 100644 --- a/packages/ocap-jsonrpc-vat/tsconfig.json +++ b/packages/ocap-jsonrpc-vat/tsconfig.json @@ -13,6 +13,7 @@ "include": [ "../../vitest.config.ts", "./src", + "./test", "./vite.config.ts", "./vitest.config.ts" ]