From d7437311377068ce70a0dd4381efcc4b478387c6 Mon Sep 17 00:00:00 2001 From: Erik Marks <25517051+rekmarks@users.noreply.github.com> Date: Wed, 7 Jan 2026 16:35:01 -0800 Subject: [PATCH 1/7] feat(omnium): Add controller architecture and CapletController Implement Phase 1.2 of the omnium plan - Define Caplet Structure: - Add modular controller architecture with POLA attenuation via makeFacet() - Add storage abstraction layer (StorageAdapter, NamespacedStorage) - Add Chrome storage adapter for platform storage - Add CapletController for managing installed caplets - Add Caplet types with superstruct validation - Wire CapletController into background.ts and expose on globalThis.omnium.caplet - Add comprehensive unit tests for all controller code - Update PLAN.md to reflect implementation --- packages/omnium-gatherum/package.json | 5 + packages/omnium-gatherum/src/background.ts | 75 +++ .../caplet/caplet-controller.test.ts | 491 ++++++++++++++++++ .../controllers/caplet/caplet-controller.ts | 298 +++++++++++ .../src/controllers/caplet/index.ts | 22 + .../src/controllers/caplet/types.test.ts | 140 +++++ .../src/controllers/caplet/types.ts | 108 ++++ .../src/controllers/facet.test.ts | 125 +++++ .../omnium-gatherum/src/controllers/facet.ts | 70 +++ .../omnium-gatherum/src/controllers/index.ts | 32 ++ .../storage/chrome-storage.test.ts | 132 +++++ .../src/controllers/storage/chrome-storage.ts | 38 ++ .../src/controllers/storage/index.ts | 3 + .../storage/namespaced-storage.test.ts | 156 ++++++ .../controllers/storage/namespaced-storage.ts | 52 ++ .../src/controllers/storage/types.ts | 88 ++++ .../omnium-gatherum/src/controllers/types.ts | 24 + packages/omnium-gatherum/src/global.d.ts | 66 +++ yarn.lock | 5 + 19 files changed, 1930 insertions(+) create mode 100644 packages/omnium-gatherum/src/controllers/caplet/caplet-controller.test.ts create mode 100644 packages/omnium-gatherum/src/controllers/caplet/caplet-controller.ts create mode 100644 packages/omnium-gatherum/src/controllers/caplet/index.ts create mode 100644 packages/omnium-gatherum/src/controllers/caplet/types.test.ts create mode 100644 packages/omnium-gatherum/src/controllers/caplet/types.ts create mode 100644 packages/omnium-gatherum/src/controllers/facet.test.ts create mode 100644 packages/omnium-gatherum/src/controllers/facet.ts create mode 100644 packages/omnium-gatherum/src/controllers/index.ts create mode 100644 packages/omnium-gatherum/src/controllers/storage/chrome-storage.test.ts create mode 100644 packages/omnium-gatherum/src/controllers/storage/chrome-storage.ts create mode 100644 packages/omnium-gatherum/src/controllers/storage/index.ts create mode 100644 packages/omnium-gatherum/src/controllers/storage/namespaced-storage.test.ts create mode 100644 packages/omnium-gatherum/src/controllers/storage/namespaced-storage.ts create mode 100644 packages/omnium-gatherum/src/controllers/storage/types.ts create mode 100644 packages/omnium-gatherum/src/controllers/types.ts diff --git a/packages/omnium-gatherum/package.json b/packages/omnium-gatherum/package.json index 161d861da..fdd737610 100644 --- a/packages/omnium-gatherum/package.json +++ b/packages/omnium-gatherum/package.json @@ -44,14 +44,19 @@ }, "dependencies": { "@endo/eventual-send": "^1.3.4", + "@endo/exo": "^1.5.12", "@metamask/kernel-browser-runtime": "workspace:^", "@metamask/kernel-shims": "workspace:^", "@metamask/kernel-ui": "workspace:^", "@metamask/kernel-utils": "workspace:^", "@metamask/logger": "workspace:^", + "@metamask/ocap-kernel": "workspace:^", "@metamask/streams": "workspace:^", + "@metamask/superstruct": "^3.2.1", + "@metamask/utils": "^11.9.0", "react": "^17.0.2", "react-dom": "^17.0.2", + "semver": "^7.7.1", "ses": "^1.14.0" }, "devDependencies": { diff --git a/packages/omnium-gatherum/src/background.ts b/packages/omnium-gatherum/src/background.ts index 4ea6aa780..4b15285e1 100644 --- a/packages/omnium-gatherum/src/background.ts +++ b/packages/omnium-gatherum/src/background.ts @@ -12,8 +12,16 @@ import type { import { delay, isJsonRpcMessage, stringify } from '@metamask/kernel-utils'; import type { JsonRpcMessage } from '@metamask/kernel-utils'; import { Logger } from '@metamask/logger'; +import type { ClusterConfig } from '@metamask/ocap-kernel'; import { ChromeRuntimeDuplexStream } from '@metamask/streams/browser'; +import { + makeChromeStorageAdapter, + makeNamespacedStorage, + makeCapletController, +} from './controllers/index.ts'; +import type { CapletManifest, LaunchResult } from './controllers/index.ts'; + defineGlobals(); const OFFSCREEN_DOCUMENT_PATH = '/offscreen.html'; @@ -111,6 +119,73 @@ async function main(): Promise { logger.info(result); }; + // Helper to get the kernel remote presence (for use with E()) + const getKernel = async (): Promise => { + return kernelP; + }; + + // Create storage adapter and namespaced storage for caplets + const storageAdapter = makeChromeStorageAdapter(); + const capletStorage = makeNamespacedStorage('caplet', storageAdapter); + + // Create CapletController with attenuated kernel access + const capletController = makeCapletController( + { logger: logger.subLogger({ tags: ['caplet'] }) }, + { + storage: capletStorage, + // Wrap launchSubcluster to return subclusterId + launchSubcluster: async ( + config: ClusterConfig, + ): Promise => { + // Get current subcluster count + const statusBefore = await E(kernelP).getStatus(); + const beforeIds = new Set( + statusBefore.subclusters.map((subcluster) => subcluster.id), + ); + + // Launch the subcluster + await E(kernelP).launchSubcluster(config); + + // Get status after and find the new subcluster + const statusAfter = await E(kernelP).getStatus(); + const newSubcluster = statusAfter.subclusters.find( + (subcluster) => !beforeIds.has(subcluster.id), + ); + + if (!newSubcluster) { + throw new Error('Failed to determine subclusterId after launch'); + } + + return { subclusterId: newSubcluster.id }; + }, + terminateSubcluster: async (subclusterId: string): Promise => { + await E(kernelP).terminateSubcluster(subclusterId); + }, + }, + ); + + Object.defineProperties(globalThis.omnium, { + ping: { + value: ping, + }, + getKernel: { + value: getKernel, + }, + caplet: { + value: harden({ + install: async (manifest: CapletManifest, bundle?: unknown) => + E(capletController).install(manifest, bundle), + uninstall: async (capletId: string) => + E(capletController).uninstall(capletId), + list: async () => E(capletController).list(), + get: async (capletId: string) => E(capletController).get(capletId), + getByService: async (serviceName: string) => + E(capletController).getByService(serviceName), + }), + }, + }); + harden(globalThis.omnium); + try { await offscreenStream.drain((message) => { if (isCapTPNotification(message)) { diff --git a/packages/omnium-gatherum/src/controllers/caplet/caplet-controller.test.ts b/packages/omnium-gatherum/src/controllers/caplet/caplet-controller.test.ts new file mode 100644 index 000000000..0da7d6b59 --- /dev/null +++ b/packages/omnium-gatherum/src/controllers/caplet/caplet-controller.test.ts @@ -0,0 +1,491 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; + +import { makeCapletController } from './caplet-controller.ts'; +import type { CapletManifest } from './types.ts'; +import type { NamespacedStorage } from '../storage/types.ts'; +import type { ControllerConfig } from '../types.ts'; + +describe('makeCapletController', () => { + const mockLogger = { + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + debug: vi.fn(), + subLogger: vi.fn().mockReturnThis(), + }; + + const mockStorage: NamespacedStorage = { + get: vi.fn(), + set: vi.fn(), + delete: vi.fn(), + has: vi.fn(), + keys: vi.fn(), + clear: vi.fn(), + }; + + const mockLaunchSubcluster = vi.fn(); + const mockTerminateSubcluster = vi.fn(); + + const config: ControllerConfig = { + logger: mockLogger as unknown as ControllerConfig['logger'], + }; + + const deps = { + storage: mockStorage, + launchSubcluster: mockLaunchSubcluster, + terminateSubcluster: mockTerminateSubcluster, + }; + + const validManifest: CapletManifest = { + id: 'com.example.test', + name: 'Test Caplet', + version: '1.0.0', + bundleSpec: 'https://example.com/bundle.json', + requestedServices: ['keyring'], + providedServices: ['signer'], + }; + + beforeEach(() => { + vi.mocked(mockStorage.has).mockResolvedValue(false); + vi.mocked(mockStorage.keys).mockResolvedValue([]); + vi.mocked(mockLaunchSubcluster).mockResolvedValue({ + subclusterId: 'subcluster-123', + }); + }); + + describe('install', () => { + it('installs a caplet successfully', async () => { + const controller = makeCapletController(config, deps); + const result = await controller.install(validManifest); + + expect(result).toStrictEqual({ + capletId: 'com.example.test', + subclusterId: 'subcluster-123', + }); + }); + + it('validates the manifest', async () => { + const controller = makeCapletController(config, deps); + const invalidManifest = { id: 'invalid' } as CapletManifest; + + await expect(controller.install(invalidManifest)).rejects.toThrow( + 'Invalid caplet manifest for invalid', + ); + }); + + it('throws if caplet already installed', async () => { + vi.mocked(mockStorage.get).mockImplementation(async (key: string) => { + if (key === 'com.example.test.manifest') { + return validManifest; + } + return undefined; + }); + + const controller = makeCapletController(config, deps); + + await expect(controller.install(validManifest)).rejects.toThrow( + 'Caplet com.example.test is already installed', + ); + }); + + it('launches subcluster with correct config', async () => { + const controller = makeCapletController(config, deps); + await controller.install(validManifest); + + expect(mockLaunchSubcluster).toHaveBeenCalledWith({ + bootstrap: 'com.example.test', + vats: { + 'com.example.test': { + bundleSpec: 'https://example.com/bundle.json', + }, + }, + }); + }); + + it('stores manifest, subclusterId, and installedAt', async () => { + vi.useFakeTimers(); + vi.setSystemTime(new Date('2024-01-15T12:00:00Z')); + + const controller = makeCapletController(config, deps); + await controller.install(validManifest); + + expect(mockStorage.set).toHaveBeenCalledWith( + 'com.example.test.manifest', + validManifest, + ); + expect(mockStorage.set).toHaveBeenCalledWith( + 'com.example.test.subclusterId', + 'subcluster-123', + ); + expect(mockStorage.set).toHaveBeenCalledWith( + 'com.example.test.installedAt', + Date.now(), + ); + + vi.useRealTimers(); + }); + + it('updates installed list', async () => { + vi.mocked(mockStorage.get).mockImplementation(async (key: string) => { + if (key === 'installed') { + return ['com.other.caplet']; + } + return undefined; + }); + + const controller = makeCapletController(config, deps); + await controller.install(validManifest); + + expect(mockStorage.set).toHaveBeenCalledWith('installed', [ + 'com.other.caplet', + 'com.example.test', + ]); + }); + + it('does not duplicate caplet id in installed list', async () => { + vi.mocked(mockStorage.get).mockImplementation(async (key: string) => { + if (key === 'installed') { + return ['com.example.test']; + } + // Return undefined for manifest to allow install to proceed + return undefined; + }); + + const controller = makeCapletController(config, deps); + await controller.install(validManifest); + + // Should not add duplicate + expect(mockStorage.set).not.toHaveBeenCalledWith('installed', [ + 'com.example.test', + 'com.example.test', + ]); + }); + + it('logs installation progress', async () => { + const controller = makeCapletController(config, deps); + await controller.install(validManifest); + + expect(mockLogger.info).toHaveBeenCalledWith( + 'Installing caplet: com.example.test', + ); + expect(mockLogger.info).toHaveBeenCalledWith( + 'Caplet com.example.test installed with subcluster subcluster-123', + ); + }); + }); + + describe('uninstall', () => { + it('uninstalls a caplet successfully', async () => { + vi.mocked(mockStorage.get).mockImplementation(async (key: string) => { + if (key === 'com.example.test.subclusterId') { + return 'subcluster-123'; + } + if (key === 'installed') { + return ['com.example.test']; + } + return undefined; + }); + + const controller = makeCapletController(config, deps); + await controller.uninstall('com.example.test'); + + expect(mockTerminateSubcluster).toHaveBeenCalledWith('subcluster-123'); + }); + + it('throws if caplet not found', async () => { + const controller = makeCapletController(config, deps); + + await expect( + controller.uninstall('com.example.notfound'), + ).rejects.toThrow('Caplet com.example.notfound not found'); + }); + + it('removes all caplet data from storage', async () => { + vi.mocked(mockStorage.get).mockImplementation(async (key: string) => { + if (key === 'com.example.test.subclusterId') { + return 'subcluster-123'; + } + if (key === 'installed') { + return ['com.example.test']; + } + return undefined; + }); + + const controller = makeCapletController(config, deps); + await controller.uninstall('com.example.test'); + + expect(mockStorage.delete).toHaveBeenCalledWith( + 'com.example.test.manifest', + ); + expect(mockStorage.delete).toHaveBeenCalledWith( + 'com.example.test.subclusterId', + ); + expect(mockStorage.delete).toHaveBeenCalledWith( + 'com.example.test.installedAt', + ); + }); + + it('updates installed list', async () => { + vi.mocked(mockStorage.get).mockImplementation(async (key: string) => { + if (key === 'com.example.test.subclusterId') { + return 'subcluster-123'; + } + if (key === 'installed') { + return ['com.other.caplet', 'com.example.test']; + } + return undefined; + }); + + const controller = makeCapletController(config, deps); + await controller.uninstall('com.example.test'); + + expect(mockStorage.set).toHaveBeenCalledWith('installed', [ + 'com.other.caplet', + ]); + }); + + it('logs uninstallation progress', async () => { + vi.mocked(mockStorage.get).mockImplementation(async (key: string) => { + if (key === 'com.example.test.subclusterId') { + return 'subcluster-123'; + } + if (key === 'installed') { + return ['com.example.test']; + } + return undefined; + }); + + const controller = makeCapletController(config, deps); + await controller.uninstall('com.example.test'); + + expect(mockLogger.info).toHaveBeenCalledWith( + 'Uninstalling caplet: com.example.test', + ); + expect(mockLogger.info).toHaveBeenCalledWith( + 'Caplet com.example.test uninstalled', + ); + }); + }); + + describe('list', () => { + it('returns empty array when no caplets installed', async () => { + const controller = makeCapletController(config, deps); + const result = await controller.list(); + + expect(result).toStrictEqual([]); + }); + + it('returns all installed caplets', async () => { + const manifest2: CapletManifest = { + ...validManifest, + id: 'com.example.test2', + name: 'Test Caplet 2', + }; + + vi.mocked(mockStorage.get).mockImplementation(async (key: string) => { + if (key === 'installed') { + return ['com.example.test', 'com.example.test2']; + } + if (key === 'com.example.test.manifest') { + return validManifest; + } + if (key === 'com.example.test.subclusterId') { + return 'subcluster-1'; + } + if (key === 'com.example.test.installedAt') { + return 1000; + } + if (key === 'com.example.test2.manifest') { + return manifest2; + } + if (key === 'com.example.test2.subclusterId') { + return 'subcluster-2'; + } + if (key === 'com.example.test2.installedAt') { + return 2000; + } + return undefined; + }); + + const controller = makeCapletController(config, deps); + const result = await controller.list(); + + expect(result).toHaveLength(2); + expect(result[0]).toStrictEqual({ + manifest: validManifest, + subclusterId: 'subcluster-1', + installedAt: 1000, + }); + expect(result[1]).toStrictEqual({ + manifest: manifest2, + subclusterId: 'subcluster-2', + installedAt: 2000, + }); + }); + + it('skips caplets with missing data', async () => { + vi.mocked(mockStorage.get).mockImplementation(async (key: string) => { + if (key === 'installed') { + return ['com.example.test', 'com.example.missing']; + } + if (key === 'com.example.test.manifest') { + return validManifest; + } + if (key === 'com.example.test.subclusterId') { + return 'subcluster-1'; + } + if (key === 'com.example.test.installedAt') { + return 1000; + } + // com.example.missing has no data + return undefined; + }); + + const controller = makeCapletController(config, deps); + const result = await controller.list(); + + expect(result).toHaveLength(1); + expect(result[0]?.manifest.id).toBe('com.example.test'); + }); + }); + + describe('get', () => { + it('returns caplet if exists', async () => { + vi.mocked(mockStorage.get).mockImplementation(async (key: string) => { + if (key === 'com.example.test.manifest') { + return validManifest; + } + if (key === 'com.example.test.subclusterId') { + return 'subcluster-123'; + } + if (key === 'com.example.test.installedAt') { + return 1705320000000; + } + return undefined; + }); + + const controller = makeCapletController(config, deps); + const result = await controller.get('com.example.test'); + + expect(result).toStrictEqual({ + manifest: validManifest, + subclusterId: 'subcluster-123', + installedAt: 1705320000000, + }); + }); + + it('returns undefined if caplet not found', async () => { + const controller = makeCapletController(config, deps); + const result = await controller.get('com.example.notfound'); + + expect(result).toBeUndefined(); + }); + + it('returns undefined and logs warning if storage data corrupted', async () => { + vi.mocked(mockStorage.get).mockImplementation(async (key: string) => { + if (key === 'com.example.test.manifest') { + return validManifest; + } + // Missing subclusterId and installedAt + return undefined; + }); + + const controller = makeCapletController(config, deps); + const result = await controller.get('com.example.test'); + + expect(result).toBeUndefined(); + expect(mockLogger.warn).toHaveBeenCalledWith( + 'Caplet com.example.test has corrupted storage data', + ); + }); + }); + + describe('getByService', () => { + it('returns caplet providing the service', async () => { + vi.mocked(mockStorage.get).mockImplementation(async (key: string) => { + if (key === 'installed') { + return ['com.example.test']; + } + if (key === 'com.example.test.manifest') { + return validManifest; // providedServices: ['signer'] + } + if (key === 'com.example.test.subclusterId') { + return 'subcluster-123'; + } + if (key === 'com.example.test.installedAt') { + return 1000; + } + return undefined; + }); + + const controller = makeCapletController(config, deps); + const result = await controller.getByService('signer'); + + expect(result).toBeDefined(); + expect(result?.manifest.id).toBe('com.example.test'); + }); + + it('returns undefined if no caplet provides the service', async () => { + vi.mocked(mockStorage.get).mockImplementation(async (key: string) => { + if (key === 'installed') { + return ['com.example.test']; + } + if (key === 'com.example.test.manifest') { + return validManifest; + } + if (key === 'com.example.test.subclusterId') { + return 'subcluster-123'; + } + if (key === 'com.example.test.installedAt') { + return 1000; + } + return undefined; + }); + + const controller = makeCapletController(config, deps); + const result = await controller.getByService('unknown-service'); + + expect(result).toBeUndefined(); + }); + + it('returns first matching caplet when multiple provide the service', async () => { + const manifest2: CapletManifest = { + ...validManifest, + id: 'com.example.test2', + name: 'Test Caplet 2', + providedServices: ['signer', 'verifier'], + }; + + vi.mocked(mockStorage.get).mockImplementation(async (key: string) => { + if (key === 'installed') { + return ['com.example.test', 'com.example.test2']; + } + if (key === 'com.example.test.manifest') { + return validManifest; + } + if (key === 'com.example.test.subclusterId') { + return 'subcluster-1'; + } + if (key === 'com.example.test.installedAt') { + return 1000; + } + if (key === 'com.example.test2.manifest') { + return manifest2; + } + if (key === 'com.example.test2.subclusterId') { + return 'subcluster-2'; + } + if (key === 'com.example.test2.installedAt') { + return 2000; + } + return undefined; + }); + + const controller = makeCapletController(config, deps); + const result = await controller.getByService('signer'); + + // Returns first match + expect(result?.manifest.id).toBe('com.example.test'); + }); + }); +}); diff --git a/packages/omnium-gatherum/src/controllers/caplet/caplet-controller.ts b/packages/omnium-gatherum/src/controllers/caplet/caplet-controller.ts new file mode 100644 index 000000000..3a4ed0ed0 --- /dev/null +++ b/packages/omnium-gatherum/src/controllers/caplet/caplet-controller.ts @@ -0,0 +1,298 @@ +import { makeDefaultExo } from '@metamask/kernel-utils/exo'; +import type { ClusterConfig } from '@metamask/ocap-kernel'; + +import type { + CapletId, + CapletManifest, + InstalledCaplet, + InstallResult, + LaunchResult, +} from './types.ts'; +import { isCapletManifest } from './types.ts'; +import type { NamespacedStorage } from '../storage/types.ts'; +import type { ControllerConfig } from '../types.ts'; + +/** + * Storage keys used by the CapletController within its namespace. + */ +const STORAGE_KEYS = { + /** List of installed caplet IDs */ + INSTALLED_LIST: 'installed', + /** Suffix for manifest storage: `${capletId}.manifest` */ + MANIFEST_SUFFIX: '.manifest', + /** Suffix for subclusterId storage: `${capletId}.subclusterId` */ + SUBCLUSTER_SUFFIX: '.subclusterId', + /** Suffix for installedAt storage: `${capletId}.installedAt` */ + INSTALLED_AT_SUFFIX: '.installedAt', +} as const; + +/** + * Generate storage key for a caplet's manifest. + * + * @param capletId - The caplet ID. + * @returns The storage key. + */ +const manifestKey = (capletId: CapletId): string => + `${capletId}${STORAGE_KEYS.MANIFEST_SUFFIX}`; + +/** + * Generate storage key for a caplet's subclusterId. + * + * @param capletId - The caplet ID. + * @returns The storage key. + */ +const subclusterKey = (capletId: CapletId): string => + `${capletId}${STORAGE_KEYS.SUBCLUSTER_SUFFIX}`; + +/** + * Generate storage key for a caplet's installedAt timestamp. + * + * @param capletId - The caplet ID. + * @returns The storage key. + */ +const installedAtKey = (capletId: CapletId): string => + `${capletId}${STORAGE_KEYS.INSTALLED_AT_SUFFIX}`; + +/** + * Methods exposed by the CapletController. + */ +export type CapletControllerMethods = { + /** + * Install a caplet. + * + * @param manifest - The caplet manifest. + * @param _bundle - The caplet bundle (currently unused, bundle loaded from bundleSpec). + * @returns The installation result. + */ + install: ( + manifest: CapletManifest, + _bundle?: unknown, + ) => Promise; + + /** + * Uninstall a caplet. + * + * @param capletId - The ID of the caplet to uninstall. + */ + uninstall: (capletId: CapletId) => Promise; + + /** + * List all installed caplets. + * + * @returns Array of installed caplets. + */ + list: () => Promise; + + /** + * Get a specific installed caplet. + * + * @param capletId - The caplet ID. + * @returns The installed caplet or undefined if not found. + */ + get: (capletId: CapletId) => Promise; + + /** + * Find a caplet that provides a specific service. + * + * @param serviceName - The service name to search for. + * @returns The installed caplet or undefined if not found. + */ + getByService: (serviceName: string) => Promise; +}; + +/** + * Dependencies for the CapletController. + * These are attenuated - only the methods needed are provided. + */ +export type CapletControllerDeps = { + /** Namespaced storage for caplet data */ + storage: NamespacedStorage; + /** Launch a subcluster for a caplet */ + launchSubcluster: (config: ClusterConfig) => Promise; + /** Terminate a caplet's subcluster */ + terminateSubcluster: (subclusterId: string) => Promise; +}; + +/** + * Create the CapletController. + * + * The CapletController manages the lifecycle of installed caplets: + * - Installing caplets (validating manifest, launching subcluster, storing metadata) + * - Uninstalling caplets (terminating subcluster, removing metadata) + * - Querying installed caplets + * + * @param config - Controller configuration. + * @param deps - Controller dependencies (attenuated for POLA). + * @returns A hardened CapletController exo. + */ +export function makeCapletController( + config: ControllerConfig, + deps: CapletControllerDeps, +): CapletControllerMethods { + const { logger } = config; + const { storage, launchSubcluster, terminateSubcluster } = deps; + + /** + * Get the list of installed caplet IDs. + * + * @returns Array of installed caplet IDs. + */ + const getInstalledIds = async (): Promise => { + const ids = await storage.get(STORAGE_KEYS.INSTALLED_LIST); + return ids ?? []; + }; + + /** + * Update the list of installed caplet IDs. + * + * @param ids - The list of caplet IDs to store. + */ + const setInstalledIds = async (ids: CapletId[]): Promise => { + await storage.set(STORAGE_KEYS.INSTALLED_LIST, ids); + }; + + /** + * Internal get implementation (to avoid `this` binding issues in exo). + * + * @param capletId - The caplet ID to retrieve. + * @returns The installed caplet or undefined if not found. + */ + const getCaplet = async ( + capletId: CapletId, + ): Promise => { + const manifest = await storage.get(manifestKey(capletId)); + if (manifest === undefined) { + return undefined; + } + + const [subclusterId, installedAt] = await Promise.all([ + storage.get(subclusterKey(capletId)), + storage.get(installedAtKey(capletId)), + ]); + + if (subclusterId === undefined || installedAt === undefined) { + // Corrupted data - manifest exists but other fields don't + logger.warn(`Caplet ${capletId} has corrupted storage data`); + return undefined; + } + + return { + manifest, + subclusterId, + installedAt, + }; + }; + + /** + * Internal list implementation (to avoid `this` binding issues in exo). + * + * @returns Array of all installed caplets. + */ + const listCaplets = async (): Promise => { + const installedIds = await getInstalledIds(); + const caplets: InstalledCaplet[] = []; + + for (const id of installedIds) { + const caplet = await getCaplet(id); + if (caplet !== undefined) { + caplets.push(caplet); + } + } + + return caplets; + }; + + return makeDefaultExo('CapletController', { + async install( + manifest: CapletManifest, + _bundle?: unknown, + ): Promise { + const { id } = manifest; + logger.info(`Installing caplet: ${id}`); + + // Validate manifest + if (!isCapletManifest(manifest)) { + throw new Error(`Invalid caplet manifest for ${id}`); + } + + // Check if already installed + const existing = await storage.get(manifestKey(id)); + if (existing !== undefined) { + throw new Error(`Caplet ${id} is already installed`); + } + + // Create cluster config for this caplet + const clusterConfig: ClusterConfig = { + bootstrap: id, + vats: { + [id]: { + bundleSpec: manifest.bundleSpec, + }, + }, + }; + + // Launch subcluster + const { subclusterId } = await launchSubcluster(clusterConfig); + + // Store caplet data + const now = Date.now(); + await Promise.all([ + storage.set(manifestKey(id), manifest), + storage.set(subclusterKey(id), subclusterId), + storage.set(installedAtKey(id), now), + ]); + + // Update installed list + const installedIds = await getInstalledIds(); + if (!installedIds.includes(id)) { + await setInstalledIds([...installedIds, id]); + } + + logger.info(`Caplet ${id} installed with subcluster ${subclusterId}`); + return { capletId: id, subclusterId }; + }, + + async uninstall(capletId: CapletId): Promise { + logger.info(`Uninstalling caplet: ${capletId}`); + + const subclusterId = await storage.get(subclusterKey(capletId)); + if (subclusterId === undefined) { + throw new Error(`Caplet ${capletId} not found`); + } + + // Terminate the subcluster + await terminateSubcluster(subclusterId); + + // Remove from storage + await Promise.all([ + storage.delete(manifestKey(capletId)), + storage.delete(subclusterKey(capletId)), + storage.delete(installedAtKey(capletId)), + ]); + + // Update installed list + const installedIds = await getInstalledIds(); + await setInstalledIds(installedIds.filter((id) => id !== capletId)); + + logger.info(`Caplet ${capletId} uninstalled`); + }, + + async list(): Promise { + return listCaplets(); + }, + + async get(capletId: CapletId): Promise { + return getCaplet(capletId); + }, + + async getByService( + serviceName: string, + ): Promise { + const caplets = await listCaplets(); + return caplets.find((caplet: InstalledCaplet) => + caplet.manifest.providedServices.includes(serviceName), + ); + }, + }); +} +harden(makeCapletController); diff --git a/packages/omnium-gatherum/src/controllers/caplet/index.ts b/packages/omnium-gatherum/src/controllers/caplet/index.ts new file mode 100644 index 000000000..e2b30889c --- /dev/null +++ b/packages/omnium-gatherum/src/controllers/caplet/index.ts @@ -0,0 +1,22 @@ +export type { + CapletId, + SemVer, + CapletManifest, + InstalledCaplet, + InstallResult, + LaunchResult, +} from './types.ts'; +export { + isCapletId, + isSemVer, + isCapletManifest, + assertCapletManifest, + CapletIdStruct, + SemVerStruct, + CapletManifestStruct, +} from './types.ts'; +export type { + CapletControllerMethods, + CapletControllerDeps, +} from './caplet-controller.ts'; +export { makeCapletController } from './caplet-controller.ts'; diff --git a/packages/omnium-gatherum/src/controllers/caplet/types.test.ts b/packages/omnium-gatherum/src/controllers/caplet/types.test.ts new file mode 100644 index 000000000..2b1138f5f --- /dev/null +++ b/packages/omnium-gatherum/src/controllers/caplet/types.test.ts @@ -0,0 +1,140 @@ +import { describe, it, expect } from 'vitest'; + +import { + isCapletId, + isSemVer, + isCapletManifest, + assertCapletManifest, +} from './types.ts'; + +describe('isCapletId', () => { + it.each([ + ['com.example.test', true], + ['org.metamask.keyring', true], + ['io.github.user.package', true], + ['a.b', true], + ['a1.b2', true], + ['test.caplet123', true], + ])('validates "%s" as %s', (value, expected) => { + expect(isCapletId(value)).toBe(expected); + }); + + it.each([ + ['', false], + ['single', false], // Must have at least 2 segments + ['com.Example.test', false], // No uppercase + ['com.123.test', false], // Segments cannot start with number + ['com..test', false], // Empty segment + ['com.test-name', false], // No hyphens + ['com.test_name', false], // No underscores + ['.com.test', false], // Cannot start with dot + ['com.test.', false], // Cannot end with dot + [123, false], // Not a string + [null, false], + [undefined, false], + [{}, false], + ])('rejects %s', (value, expected) => { + expect(isCapletId(value)).toBe(expected); + }); +}); + +describe('isSemVer', () => { + it.each([ + ['1.0.0', true], + ['0.0.1', true], + ['10.20.30', true], + ['1.0.0-alpha', true], + ['1.0.0-alpha.1', true], + ['0.0.0', true], + ['999.999.999', true], + ['1.2.3-0', true], + ])('validates "%s" as %s', (value, expected) => { + expect(isSemVer(value)).toBe(expected); + }); + + it.each([ + ['1.0', false], + ['1', false], + ['v1.0.0', false], // No 'v' prefix + ['1.0.0.0', false], + ['', false], + ['not-a-version', false], + ['1.0.0+build.123', false], // Build metadata not supported (semver strips it) + ['1.0.0-beta+build', false], // Build metadata not supported + [123, false], + [null, false], + [undefined, false], + ])('rejects %s', (value, expected) => { + expect(isSemVer(value)).toBe(expected); + }); +}); + +describe('isCapletManifest', () => { + const validManifest = { + id: 'com.example.test', + name: 'Test Caplet', + version: '1.0.0', + bundleSpec: 'https://example.com/bundle.json', + requestedServices: ['keyring'], + providedServices: ['signer'], + }; + + it('validates a complete manifest', () => { + expect(isCapletManifest(validManifest)).toBe(true); + }); + + it('validates a manifest with empty service arrays', () => { + const manifest = { + ...validManifest, + requestedServices: [], + providedServices: [], + }; + expect(isCapletManifest(manifest)).toBe(true); + }); + + it('rejects manifest with invalid id', () => { + expect(isCapletManifest({ ...validManifest, id: 'invalid' })).toBe(false); + }); + + it('rejects manifest with invalid version', () => { + expect(isCapletManifest({ ...validManifest, version: '1.0' })).toBe(false); + }); + + it('rejects manifest missing required field', () => { + const { name: _name, ...missingName } = validManifest; + expect(isCapletManifest(missingName)).toBe(false); + }); + + it('rejects null', () => { + expect(isCapletManifest(null)).toBe(false); + }); + + it('rejects non-object', () => { + expect(isCapletManifest('string')).toBe(false); + }); +}); + +describe('assertCapletManifest', () => { + const validManifest = { + id: 'com.example.test', + name: 'Test Caplet', + version: '1.0.0', + bundleSpec: 'https://example.com/bundle.json', + requestedServices: [], + providedServices: [], + }; + + it('does not throw for valid manifest', () => { + expect(() => assertCapletManifest(validManifest)).not.toThrow(); + }); + + it('throws for invalid manifest', () => { + expect(() => assertCapletManifest({ id: 'bad' })).toThrow( + 'Invalid CapletManifest', + ); + }); + + it('throws for null', () => { + expect(() => assertCapletManifest(null)).toThrow('Invalid CapletManifest'); + }); +}); diff --git a/packages/omnium-gatherum/src/controllers/caplet/types.ts b/packages/omnium-gatherum/src/controllers/caplet/types.ts new file mode 100644 index 000000000..cdf201be7 --- /dev/null +++ b/packages/omnium-gatherum/src/controllers/caplet/types.ts @@ -0,0 +1,108 @@ +import { array, define, is, object, string } from '@metamask/superstruct'; +import type { Infer } from '@metamask/superstruct'; +import semverValid from 'semver/functions/valid'; + +/** + * Unique identifier for a Caplet. + * Uses reverse domain notation (e.g., "com.example.bitcoin-signer"). + */ +export type CapletId = string; + +/** + * Validate CapletId format. + * Requires lowercase alphanumeric segments separated by dots, minimum 2 segments. + * + * @param value - The value to validate. + * @returns True if valid CapletId format. + */ +export const isCapletId = (value: unknown): value is CapletId => + typeof value === 'string' && + value.length > 0 && + /^[a-z][a-z0-9]*(\.[a-z][a-z0-9]*)+$/u.test(value); + +export const CapletIdStruct = define('CapletId', isCapletId); + +/** + * Semantic version string (e.g., "1.0.0"). + */ +export type SemVer = string; + +/** + * Validate SemVer format using the semver package. + * Requires strict format without 'v' prefix (e.g., "1.0.0" not "v1.0.0"). + * + * @param value - The value to validate. + * @returns True if valid SemVer format. + */ +export const isSemVer = (value: unknown): value is SemVer => + typeof value === 'string' && + // semver.valid() is lenient and strips 'v' prefix, so check that cleaned value equals original + semverValid(value) === value; + +export const SemVerStruct = define('SemVer', isSemVer); + +/** + * Superstruct schema for validating CapletManifest objects. + */ +export const CapletManifestStruct = object({ + id: CapletIdStruct, + name: string(), + version: SemVerStruct, + bundleSpec: string(), + requestedServices: array(string()), + providedServices: array(string()), +}); + +/** + * Metadata that defines a Caplet's identity, dependencies, and capabilities. + */ +export type CapletManifest = Infer; + +/** + * Type guard for CapletManifest validation. + * + * @param value - The value to validate. + * @returns True if the value is a valid CapletManifest. + */ +export const isCapletManifest = (value: unknown): value is CapletManifest => + is(value, CapletManifestStruct); + +/** + * Assert that a value is a valid CapletManifest. + * + * @param value - The value to validate. + * @throws If the value is not a valid CapletManifest. + */ +export function assertCapletManifest( + value: unknown, +): asserts value is CapletManifest { + if (!isCapletManifest(value)) { + throw new Error('Invalid CapletManifest'); + } +} + +/** + * Record for an installed Caplet. + * Combines manifest with runtime identifiers. + */ +export type InstalledCaplet = { + manifest: CapletManifest; + subclusterId: string; + installedAt: number; +}; + +/** + * Result of installing a Caplet. + */ +export type InstallResult = { + capletId: CapletId; + subclusterId: string; +}; + +/** + * Result of launching a subcluster. + * This is the interface expected by CapletController's deps. + */ +export type LaunchResult = { + subclusterId: string; +}; diff --git a/packages/omnium-gatherum/src/controllers/facet.test.ts b/packages/omnium-gatherum/src/controllers/facet.test.ts new file mode 100644 index 000000000..7cb784897 --- /dev/null +++ b/packages/omnium-gatherum/src/controllers/facet.test.ts @@ -0,0 +1,125 @@ +import { describe, it, expect, vi } from 'vitest'; + +import { makeFacet } from './facet.ts'; + +describe('makeFacet', () => { + const makeSourceObject = () => ({ + method1: vi.fn().mockReturnValue('result1'), + method2: vi.fn().mockReturnValue('result2'), + method3: vi.fn().mockReturnValue('result3'), + asyncMethod: vi.fn().mockResolvedValue('asyncResult'), + }); + + it('creates a facet with only specified methods', () => { + const source = makeSourceObject(); + + const facet = makeFacet('TestFacet', source, ['method1', 'method2']); + + expect(facet.method1).toBeDefined(); + expect(facet.method2).toBeDefined(); + expect((facet as Record).method3).toBeUndefined(); + expect((facet as Record).asyncMethod).toBeUndefined(); + }); + + it('facet methods call the source methods', () => { + const source = makeSourceObject(); + + const facet = makeFacet('TestFacet', source, ['method1']); + facet.method1(); + + expect(source.method1).toHaveBeenCalledOnce(); + }); + + it('facet methods return the same result as source', () => { + const source = makeSourceObject(); + + const facet = makeFacet('TestFacet', source, ['method1']); + const result = facet.method1(); + + expect(result).toBe('result1'); + }); + + it('facet methods pass arguments to source', () => { + const source = makeSourceObject(); + + const facet = makeFacet('TestFacet', source, ['method1']); + facet.method1('arg1', 'arg2'); + + expect(source.method1).toHaveBeenCalledWith('arg1', 'arg2'); + }); + + it('works with async methods', async () => { + const source = makeSourceObject(); + + const facet = makeFacet('TestFacet', source, ['asyncMethod']); + const result = await facet.asyncMethod(); + + expect(result).toBe('asyncResult'); + expect(source.asyncMethod).toHaveBeenCalledOnce(); + }); + + it('creates facet with single method', () => { + const source = makeSourceObject(); + + const facet = makeFacet('SingleMethodFacet', source, ['method1']); + + expect(facet.method1).toBeDefined(); + // Verify only the specified method is accessible + expect((facet as Record).method2).toBeUndefined(); + expect((facet as Record).method3).toBeUndefined(); + }); + + it('creates facet with all methods', () => { + const source = makeSourceObject(); + + const facet = makeFacet('AllMethodsFacet', source, [ + 'method1', + 'method2', + 'method3', + 'asyncMethod', + ]); + + expect(facet.method1).toBeDefined(); + expect(facet.method2).toBeDefined(); + expect(facet.method3).toBeDefined(); + expect(facet.asyncMethod).toBeDefined(); + }); + + it('throws when method does not exist on source', () => { + const source = makeSourceObject(); + + expect(() => + makeFacet('TestFacet', source, ['nonExistent' as keyof typeof source]), + ).toThrow( + "makeFacet: Method 'nonExistent' not found on source or is not a function", + ); + }); + + it('throws when property is not a function', () => { + const source = { + method1: vi.fn(), + notAMethod: 'string value', + }; + + expect(() => + // @ts-expect-error Destructive testing + makeFacet('TestFacet', source, ['notAMethod' as keyof typeof source]), + ).toThrow( + "makeFacet: Method 'notAMethod' not found on source or is not a function", + ); + }); + + it('preserves this context when methods use it', () => { + const source = { + value: 42, + getValue(this: { value: number }): number { + return this.value; + }, + }; + + const facet = makeFacet('TestFacet', source, ['getValue']); + const result = facet.getValue(); + + expect(result).toBe(42); + }); +}); diff --git a/packages/omnium-gatherum/src/controllers/facet.ts b/packages/omnium-gatherum/src/controllers/facet.ts new file mode 100644 index 000000000..95e8a1338 --- /dev/null +++ b/packages/omnium-gatherum/src/controllers/facet.ts @@ -0,0 +1,70 @@ +import type { Methods, MethodGuard } from '@endo/exo'; +import { makeDefaultExo } from '@metamask/kernel-utils/exo'; + +// RemotableMethodName from @endo/pass-style is string | symbol +type MethodKeys = Extract< + { + [Key in keyof Source]: Source[Key] extends CallableFunction ? Key : never; + }[keyof Source], + keyof MethodGuard +>; + +type BoundMethod = Func extends CallableFunction + ? OmitThisParameter + : never; + +type FacetMethods> = Methods & { + [Key in MethodNames]: BoundMethod; +}; + +/** + * Create an attenuated facet of a source object that exposes only specific methods. + * + * This enforces POLA (Principle of Least Authority) by allowing Controller A + * to receive only the methods it needs from Controller B. + * + * @param name - Name for the facet (used in debugging/logging). + * @param source - The source object containing methods. + * @param methodNames - Array of method names to expose. + * @returns A hardened facet exo with only the specified methods. + * @example + * ```typescript + * // StorageController exposes full interface internally + * const storageController = makeStorageController(config); + * + * // CapletController only needs get/set, not clear/getAll + * const storageFacet = makeFacet('CapletStorage', storageController, ['get', 'set']); + * const capletController = makeCapletController({ storage: storageFacet }); + * ``` + */ +export function makeFacet< + Source extends Record, + MethodNames extends MethodKeys, +>( + name: string, + source: Source, + methodNames: readonly MethodNames[], +): FacetMethods { + const methods: Partial> = {}; + + for (const methodName of methodNames) { + const method = source[methodName]; + if (typeof method !== 'function') { + throw new Error( + `makeFacet: Method '${String( + methodName, + )}' not found on source or is not a function`, + ); + } + // Bind the method to preserve 'this' context if needed + methods[methodName] = (method as CallableFunction).bind( + source, + ) as BoundMethod as FacetMethods< + Source, + MethodNames + >[MethodNames]; + } + + return makeDefaultExo(name, methods as FacetMethods); +} +harden(makeFacet); diff --git a/packages/omnium-gatherum/src/controllers/index.ts b/packages/omnium-gatherum/src/controllers/index.ts new file mode 100644 index 000000000..25b792d4b --- /dev/null +++ b/packages/omnium-gatherum/src/controllers/index.ts @@ -0,0 +1,32 @@ +// Base types +export type { ControllerConfig, FacetOf } from './types.ts'; +export { makeFacet } from './facet.ts'; + +// Storage +export type { NamespacedStorage, StorageAdapter } from './storage/index.ts'; +export { + makeChromeStorageAdapter, + makeNamespacedStorage, +} from './storage/index.ts'; + +// Caplet +export type { + CapletId, + SemVer, + CapletManifest, + InstalledCaplet, + InstallResult, + LaunchResult, + CapletControllerMethods, + CapletControllerDeps, +} from './caplet/index.ts'; +export { + isCapletId, + isSemVer, + isCapletManifest, + assertCapletManifest, + CapletIdStruct, + SemVerStruct, + CapletManifestStruct, + makeCapletController, +} from './caplet/index.ts'; diff --git a/packages/omnium-gatherum/src/controllers/storage/chrome-storage.test.ts b/packages/omnium-gatherum/src/controllers/storage/chrome-storage.test.ts new file mode 100644 index 000000000..403fe7dcb --- /dev/null +++ b/packages/omnium-gatherum/src/controllers/storage/chrome-storage.test.ts @@ -0,0 +1,132 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; + +import { makeChromeStorageAdapter } from './chrome-storage.ts'; + +describe('makeChromeStorageAdapter', () => { + const mockStorage = { + get: vi.fn().mockResolvedValue({}), + set: vi.fn(), + remove: vi.fn(), + }; + + beforeEach(() => { + mockStorage.get.mockResolvedValue({}); + }); + + describe('get', () => { + it('returns value for existing key', async () => { + mockStorage.get.mockResolvedValue({ testKey: 'testValue' }); + + const adapter = makeChromeStorageAdapter( + mockStorage as unknown as chrome.storage.StorageArea, + ); + const result = await adapter.get('testKey'); + + expect(result).toBe('testValue'); + expect(mockStorage.get).toHaveBeenCalledWith('testKey'); + }); + + it('returns undefined for non-existent key', async () => { + mockStorage.get.mockResolvedValue({}); + + const adapter = makeChromeStorageAdapter( + mockStorage as unknown as chrome.storage.StorageArea, + ); + const result = await adapter.get('nonExistent'); + + expect(result).toBeUndefined(); + }); + + it('returns complex objects', async () => { + const complexValue = { nested: { data: [1, 2, 3] } }; + mockStorage.get.mockResolvedValue({ complex: complexValue }); + + const adapter = makeChromeStorageAdapter( + mockStorage as unknown as chrome.storage.StorageArea, + ); + const result = await adapter.get('complex'); + + expect(result).toStrictEqual(complexValue); + }); + }); + + describe('set', () => { + it('sets a value', async () => { + const adapter = makeChromeStorageAdapter( + mockStorage as unknown as chrome.storage.StorageArea, + ); + await adapter.set('key', 'value'); + + expect(mockStorage.set).toHaveBeenCalledWith({ key: 'value' }); + }); + + it('sets complex objects', async () => { + const complexValue = { nested: { data: [1, 2, 3] } }; + + const adapter = makeChromeStorageAdapter( + mockStorage as unknown as chrome.storage.StorageArea, + ); + await adapter.set('complex', complexValue); + + expect(mockStorage.set).toHaveBeenCalledWith({ complex: complexValue }); + }); + }); + + describe('delete', () => { + it('deletes a key', async () => { + const adapter = makeChromeStorageAdapter( + mockStorage as unknown as chrome.storage.StorageArea, + ); + await adapter.delete('keyToDelete'); + + expect(mockStorage.remove).toHaveBeenCalledWith('keyToDelete'); + }); + }); + + describe('keys', () => { + it('returns all keys when no prefix provided', async () => { + mockStorage.get.mockResolvedValue({ + key1: 'value1', + key2: 'value2', + other: 'value3', + }); + + const adapter = makeChromeStorageAdapter( + mockStorage as unknown as chrome.storage.StorageArea, + ); + const result = await adapter.keys(); + + expect(result).toStrictEqual(['key1', 'key2', 'other']); + expect(mockStorage.get).toHaveBeenCalledWith(null); + }); + + it('filters keys by prefix', async () => { + mockStorage.get.mockResolvedValue({ + 'prefix.key1': 'value1', + 'prefix.key2': 'value2', + other: 'value3', + }); + + const adapter = makeChromeStorageAdapter( + mockStorage as unknown as chrome.storage.StorageArea, + ); + const result = await adapter.keys('prefix.'); + + expect(result).toStrictEqual(['prefix.key1', 'prefix.key2']); + }); + + it('returns empty array when no keys match prefix', async () => { + mockStorage.get.mockResolvedValue({ + key1: 'value1', + key2: 'value2', + }); + + const adapter = makeChromeStorageAdapter( + mockStorage as unknown as chrome.storage.StorageArea, + ); + const result = await adapter.keys('nonexistent.'); + + expect(result).toStrictEqual([]); + }); + }); +}); diff --git a/packages/omnium-gatherum/src/controllers/storage/chrome-storage.ts b/packages/omnium-gatherum/src/controllers/storage/chrome-storage.ts new file mode 100644 index 000000000..4c0134757 --- /dev/null +++ b/packages/omnium-gatherum/src/controllers/storage/chrome-storage.ts @@ -0,0 +1,38 @@ +import type { Json } from '@metamask/utils'; + +import type { StorageAdapter } from './types.ts'; + +/** + * Create a storage adapter backed by Chrome Storage API. + * + * @param storage - The Chrome storage area to use (defaults to chrome.storage.local). + * @returns A hardened StorageAdapter instance. + */ +export function makeChromeStorageAdapter( + storage: chrome.storage.StorageArea = chrome.storage.local, +): StorageAdapter { + return harden({ + async get(key: string): Promise { + const result = await storage.get(key); + return result[key] as Value | undefined; + }, + + async set(key: string, value: Json): Promise { + await storage.set({ [key]: value }); + }, + + async delete(key: string): Promise { + await storage.remove(key); + }, + + async keys(prefix?: string): Promise { + const all = await storage.get(null); + const allKeys = Object.keys(all); + if (prefix === undefined) { + return allKeys; + } + return allKeys.filter((k) => k.startsWith(prefix)); + }, + }); +} +harden(makeChromeStorageAdapter); diff --git a/packages/omnium-gatherum/src/controllers/storage/index.ts b/packages/omnium-gatherum/src/controllers/storage/index.ts new file mode 100644 index 000000000..5d9628d33 --- /dev/null +++ b/packages/omnium-gatherum/src/controllers/storage/index.ts @@ -0,0 +1,3 @@ +export type { NamespacedStorage, StorageAdapter } from './types.ts'; +export { makeChromeStorageAdapter } from './chrome-storage.ts'; +export { makeNamespacedStorage } from './namespaced-storage.ts'; diff --git a/packages/omnium-gatherum/src/controllers/storage/namespaced-storage.test.ts b/packages/omnium-gatherum/src/controllers/storage/namespaced-storage.test.ts new file mode 100644 index 000000000..b427b63fe --- /dev/null +++ b/packages/omnium-gatherum/src/controllers/storage/namespaced-storage.test.ts @@ -0,0 +1,156 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; + +import { makeNamespacedStorage } from './namespaced-storage.ts'; +import type { StorageAdapter } from './types.ts'; + +describe('makeNamespacedStorage', () => { + const mockAdapter: StorageAdapter = { + get: vi.fn(), + set: vi.fn(), + delete: vi.fn(), + keys: vi.fn(), + }; + + beforeEach(() => { + vi.clearAllMocks(); + vi.mocked(mockAdapter.get).mockResolvedValue(undefined); + vi.mocked(mockAdapter.set).mockResolvedValue(undefined); + vi.mocked(mockAdapter.delete).mockResolvedValue(undefined); + vi.mocked(mockAdapter.keys).mockResolvedValue([]); + }); + + describe('get', () => { + it('prefixes key with namespace', async () => { + vi.mocked(mockAdapter.get).mockResolvedValue('value'); + + const storage = makeNamespacedStorage('caplet', mockAdapter); + const result = await storage.get('myKey'); + + expect(result).toBe('value'); + expect(mockAdapter.get).toHaveBeenCalledWith('caplet.myKey'); + }); + + it('returns undefined for non-existent key', async () => { + vi.mocked(mockAdapter.get).mockResolvedValue(undefined); + + const storage = makeNamespacedStorage('caplet', mockAdapter); + const result = await storage.get('nonExistent'); + + expect(result).toBeUndefined(); + }); + }); + + describe('set', () => { + it('prefixes key with namespace', async () => { + const storage = makeNamespacedStorage('caplet', mockAdapter); + await storage.set('myKey', 'myValue'); + + expect(mockAdapter.set).toHaveBeenCalledWith('caplet.myKey', 'myValue'); + }); + + it('handles complex values', async () => { + const complexValue = { nested: { data: [1, 2, 3] } }; + + const storage = makeNamespacedStorage('caplet', mockAdapter); + await storage.set('complex', complexValue); + + expect(mockAdapter.set).toHaveBeenCalledWith( + 'caplet.complex', + complexValue, + ); + }); + }); + + describe('delete', () => { + it('prefixes key with namespace', async () => { + const storage = makeNamespacedStorage('caplet', mockAdapter); + await storage.delete('myKey'); + + expect(mockAdapter.delete).toHaveBeenCalledWith('caplet.myKey'); + }); + }); + + describe('has', () => { + it('returns true when key exists', async () => { + vi.mocked(mockAdapter.get).mockResolvedValue('value'); + + const storage = makeNamespacedStorage('caplet', mockAdapter); + const result = await storage.has('myKey'); + + expect(result).toBe(true); + expect(mockAdapter.get).toHaveBeenCalledWith('caplet.myKey'); + }); + + it('returns false when key does not exist', async () => { + vi.mocked(mockAdapter.get).mockResolvedValue(undefined); + + const storage = makeNamespacedStorage('caplet', mockAdapter); + const result = await storage.has('nonExistent'); + + expect(result).toBe(false); + }); + }); + + describe('keys', () => { + it('returns keys with namespace prefix stripped', async () => { + vi.mocked(mockAdapter.keys).mockResolvedValue([ + 'caplet.key1', + 'caplet.key2', + 'caplet.nested.key', + ]); + + const storage = makeNamespacedStorage('caplet', mockAdapter); + const result = await storage.keys(); + + expect(result).toStrictEqual(['key1', 'key2', 'nested.key']); + expect(mockAdapter.keys).toHaveBeenCalledWith('caplet.'); + }); + + it('returns empty array when no keys in namespace', async () => { + vi.mocked(mockAdapter.keys).mockResolvedValue([]); + + const storage = makeNamespacedStorage('caplet', mockAdapter); + const result = await storage.keys(); + + expect(result).toStrictEqual([]); + }); + }); + + describe('clear', () => { + it('deletes all keys in namespace', async () => { + vi.mocked(mockAdapter.keys).mockResolvedValue([ + 'caplet.key1', + 'caplet.key2', + ]); + + const storage = makeNamespacedStorage('caplet', mockAdapter); + await storage.clear(); + + expect(mockAdapter.delete).toHaveBeenCalledTimes(2); + expect(mockAdapter.delete).toHaveBeenCalledWith('caplet.key1'); + expect(mockAdapter.delete).toHaveBeenCalledWith('caplet.key2'); + }); + + it('does nothing when namespace is empty', async () => { + vi.mocked(mockAdapter.keys).mockResolvedValue([]); + + const storage = makeNamespacedStorage('caplet', mockAdapter); + await storage.clear(); + + expect(mockAdapter.delete).not.toHaveBeenCalled(); + }); + }); + + describe('namespace isolation', () => { + it('uses different prefixes for different namespaces', async () => { + const storage1 = makeNamespacedStorage('caplet', mockAdapter); + const storage2 = makeNamespacedStorage('service', mockAdapter); + + await storage1.set('key', 'value1'); + await storage2.set('key', 'value2'); + + expect(mockAdapter.set).toHaveBeenCalledWith('caplet.key', 'value1'); + expect(mockAdapter.set).toHaveBeenCalledWith('service.key', 'value2'); + }); + }); +}); diff --git a/packages/omnium-gatherum/src/controllers/storage/namespaced-storage.ts b/packages/omnium-gatherum/src/controllers/storage/namespaced-storage.ts new file mode 100644 index 000000000..51e0c3eae --- /dev/null +++ b/packages/omnium-gatherum/src/controllers/storage/namespaced-storage.ts @@ -0,0 +1,52 @@ +import type { Json } from '@metamask/utils'; + +import type { NamespacedStorage, StorageAdapter } from './types.ts'; + +/** + * Create a namespaced storage interface. + * All operations are scoped to the given namespace prefix. + * + * @param namespace - The namespace prefix for all keys. + * @param adapter - The underlying storage adapter. + * @returns A hardened NamespacedStorage instance. + */ +export function makeNamespacedStorage( + namespace: string, + adapter: StorageAdapter, +): NamespacedStorage { + const prefix = `${namespace}.`; + + const buildKey = (key: string): string => `${prefix}${key}`; + + const stripPrefix = (fullKey: string): string => fullKey.slice(prefix.length); + + return harden({ + async get(key: string): Promise { + return adapter.get(buildKey(key)); + }, + + async set(key: string, value: Json): Promise { + await adapter.set(buildKey(key), value); + }, + + async delete(key: string): Promise { + await adapter.delete(buildKey(key)); + }, + + async has(key: string): Promise { + const value = await adapter.get(buildKey(key)); + return value !== undefined; + }, + + async keys(): Promise { + const allKeys = await adapter.keys(prefix); + return allKeys.map(stripPrefix); + }, + + async clear(): Promise { + const allKeys = await this.keys(); + await Promise.all(allKeys.map(async (key) => this.delete(key))); + }, + }); +} +harden(makeNamespacedStorage); diff --git a/packages/omnium-gatherum/src/controllers/storage/types.ts b/packages/omnium-gatherum/src/controllers/storage/types.ts new file mode 100644 index 000000000..dab4a14a4 --- /dev/null +++ b/packages/omnium-gatherum/src/controllers/storage/types.ts @@ -0,0 +1,88 @@ +import type { Json } from '@metamask/utils'; + +/** + * Low-level storage adapter interface. + * Wraps platform-specific storage APIs (e.g., chrome.storage.local). + */ +export type StorageAdapter = { + /** + * Get a value from storage. + * + * @param key - The storage key. + * @returns The stored value, or undefined if not found. + */ + get: (key: string) => Promise; + + /** + * Set a value in storage. + * + * @param key - The storage key. + * @param value - The value to store. + */ + set: (key: string, value: Json) => Promise; + + /** + * Delete a value from storage. + * + * @param key - The storage key. + */ + delete: (key: string) => Promise; + + /** + * Get all keys matching a prefix. + * + * @param prefix - Optional prefix to filter keys. + * @returns Array of matching keys. + */ + keys: (prefix?: string) => Promise; +}; + +/** + * Storage interface bound to a specific namespace. + * Controllers receive this instead of raw storage access. + * Keys are automatically prefixed with the namespace. + */ +export type NamespacedStorage = { + /** + * Get a value from the namespaced storage. + * + * @param key - The key within this namespace. + * @returns The stored value, or undefined if not found. + */ + get: (key: string) => Promise; + + /** + * Set a value in the namespaced storage. + * + * @param key - The key within this namespace. + * @param value - The value to store. + */ + set: (key: string, value: Json) => Promise; + + /** + * Delete a value from the namespaced storage. + * + * @param key - The key within this namespace. + */ + delete: (key: string) => Promise; + + /** + * Check if a key exists in the namespaced storage. + * + * @param key - The key within this namespace. + * @returns True if the key exists. + */ + has: (key: string) => Promise; + + /** + * Get all keys within this namespace. + * + * @returns Array of keys (without namespace prefix). + */ + keys: () => Promise; + + /** + * Clear all values in this namespace. + */ + clear: () => Promise; +}; diff --git a/packages/omnium-gatherum/src/controllers/types.ts b/packages/omnium-gatherum/src/controllers/types.ts new file mode 100644 index 000000000..2c4cfc890 --- /dev/null +++ b/packages/omnium-gatherum/src/controllers/types.ts @@ -0,0 +1,24 @@ +import type { Methods } from '@endo/exo'; +import type { Logger } from '@metamask/logger'; + +/** + * Configuration passed to all controllers during initialization. + */ +export type ControllerConfig = { + logger: Logger; +}; + +/** + * Type helper for defining facet interfaces. + * Extracts a subset of methods from a controller type for POLA attenuation. + * + * @example + * ```typescript + * type StorageReadFacet = FacetOf; + * type StorageWriteFacet = FacetOf; + * ``` + */ +export type FacetOf< + TController extends Methods, + TMethodNames extends keyof TController, +> = Pick; diff --git a/packages/omnium-gatherum/src/global.d.ts b/packages/omnium-gatherum/src/global.d.ts index a275d71d9..7e1d58bf2 100644 --- a/packages/omnium-gatherum/src/global.d.ts +++ b/packages/omnium-gatherum/src/global.d.ts @@ -1,5 +1,11 @@ import type { KernelFacade } from '@metamask/kernel-browser-runtime'; +import type { + CapletManifest, + InstalledCaplet, + InstallResult, +} from './controllers/index.ts'; + // Type declarations for omnium dev console API. declare global { /** @@ -33,6 +39,66 @@ declare global { * ``` */ getKernel: () => Promise; + + /** + * Caplet management API. + */ + caplet: { + /** + * Install a caplet. + * + * @param manifest - The caplet manifest. + * @param bundle - Optional bundle (currently unused). + * @returns The installation result. + * @example + * ```typescript + * const result = await omnium.caplet.install({ + * id: 'com.example.test', + * name: 'Test Caplet', + * version: '1.0.0', + * bundleSpec: '/path/to/bundle.json', + * requestedServices: [], + * providedServices: ['test'], + * }); + * ``` + */ + install: ( + manifest: CapletManifest, + bundle?: unknown, + ) => Promise; + + /** + * Uninstall a caplet. + * + * @param capletId - The ID of the caplet to uninstall. + */ + uninstall: (capletId: string) => Promise; + + /** + * List all installed caplets. + * + * @returns Array of installed caplets. + */ + list: () => Promise; + + /** + * Get a specific installed caplet. + * + * @param capletId - The caplet ID. + * @returns The installed caplet or undefined if not found. + */ + get: (capletId: string) => Promise; + + /** + * Find a caplet that provides a specific service. + * + * @param serviceName - The service name to search for. + * @returns The installed caplet or undefined if not found. + */ + getByService: ( + serviceName: string, + ) => Promise; + }; }; } diff --git a/yarn.lock b/yarn.lock index 8fdc13dfa..cc6e6bc65 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3919,6 +3919,7 @@ __metadata: dependencies: "@arethetypeswrong/cli": "npm:^0.17.4" "@endo/eventual-send": "npm:^1.3.4" + "@endo/exo": "npm:^1.5.12" "@metamask/auto-changelog": "npm:^5.3.0" "@metamask/eslint-config": "npm:^15.0.0" "@metamask/eslint-config-nodejs": "npm:^15.0.0" @@ -3928,7 +3929,10 @@ __metadata: "@metamask/kernel-ui": "workspace:^" "@metamask/kernel-utils": "workspace:^" "@metamask/logger": "workspace:^" + "@metamask/ocap-kernel": "workspace:^" "@metamask/streams": "workspace:^" + "@metamask/superstruct": "npm:^3.2.1" + "@metamask/utils": "npm:^11.9.0" "@ocap/cli": "workspace:^" "@ocap/repo-tools": "workspace:^" "@playwright/test": "npm:^1.54.2" @@ -3958,6 +3962,7 @@ __metadata: react: "npm:^17.0.2" react-dom: "npm:^17.0.2" rimraf: "npm:^6.0.1" + semver: "npm:^7.7.1" ses: "npm:^1.14.0" tsx: "npm:^4.20.6" turbo: "npm:^2.5.6" From ac3f0a9554e1b9ee36c7d5db236fd46f2ab6ca80 Mon Sep 17 00:00:00 2001 From: Erik Marks <25517051+rekmarks@users.noreply.github.com> Date: Thu, 8 Jan 2026 10:22:46 -0800 Subject: [PATCH 2/7] refactor(omnium): Simplify CapletController state structure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Consolidate CapletControllerState from multiple top-level keys (installed, manifests, subclusters, installedAt) into a single `caplets: Record` structure. Changes: - Add ControllerStorage abstraction using Immer for state management - Controllers work with typed state object instead of storage keys - Only modified top-level keys are persisted (via Immer patches) - Remove state corruption checks (no longer possible with atomic storage) - Fix makeFacet type - use string | symbol instead of keyof MethodGuard - Update PLAN.md to reflect new storage architecture 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- packages/omnium-gatherum/package.json | 1 + packages/omnium-gatherum/src/background.ts | 19 +- .../caplet/caplet-controller.test.ts | 543 +++++++++--------- .../controllers/caplet/caplet-controller.ts | 156 +---- .../src/controllers/caplet/index.ts | 1 + .../omnium-gatherum/src/controllers/facet.ts | 17 +- .../omnium-gatherum/src/controllers/index.ts | 9 +- .../storage/controller-storage.test.ts | 336 +++++++++++ .../controllers/storage/controller-storage.ts | 224 ++++++++ .../src/controllers/storage/index.ts | 5 + yarn.lock | 8 + 11 files changed, 922 insertions(+), 397 deletions(-) create mode 100644 packages/omnium-gatherum/src/controllers/storage/controller-storage.test.ts create mode 100644 packages/omnium-gatherum/src/controllers/storage/controller-storage.ts diff --git a/packages/omnium-gatherum/package.json b/packages/omnium-gatherum/package.json index fdd737610..ee8f0e8b7 100644 --- a/packages/omnium-gatherum/package.json +++ b/packages/omnium-gatherum/package.json @@ -54,6 +54,7 @@ "@metamask/streams": "workspace:^", "@metamask/superstruct": "^3.2.1", "@metamask/utils": "^11.9.0", + "immer": "^10.1.1", "react": "^17.0.2", "react-dom": "^17.0.2", "semver": "^7.7.1", diff --git a/packages/omnium-gatherum/src/background.ts b/packages/omnium-gatherum/src/background.ts index 4b15285e1..2c5e62fde 100644 --- a/packages/omnium-gatherum/src/background.ts +++ b/packages/omnium-gatherum/src/background.ts @@ -17,10 +17,14 @@ import { ChromeRuntimeDuplexStream } from '@metamask/streams/browser'; import { makeChromeStorageAdapter, - makeNamespacedStorage, + makeControllerStorage, makeCapletController, } from './controllers/index.ts'; -import type { CapletManifest, LaunchResult } from './controllers/index.ts'; +import type { + CapletControllerState, + CapletManifest, + LaunchResult, +} from './controllers/index.ts'; defineGlobals(); @@ -124,9 +128,16 @@ async function main(): Promise { return kernelP; }; - // Create storage adapter and namespaced storage for caplets + // Create storage adapter and state storage for caplets const storageAdapter = makeChromeStorageAdapter(); - const capletStorage = makeNamespacedStorage('caplet', storageAdapter); + const defaultCapletState: CapletControllerState = { + caplets: {}, + }; + const capletStorage = await makeControllerStorage({ + namespace: 'caplet', + adapter: storageAdapter, + defaultState: defaultCapletState, + }); // Create CapletController with attenuated kernel access const capletController = makeCapletController( diff --git a/packages/omnium-gatherum/src/controllers/caplet/caplet-controller.test.ts b/packages/omnium-gatherum/src/controllers/caplet/caplet-controller.test.ts index 0da7d6b59..23b99df3a 100644 --- a/packages/omnium-gatherum/src/controllers/caplet/caplet-controller.test.ts +++ b/packages/omnium-gatherum/src/controllers/caplet/caplet-controller.test.ts @@ -1,10 +1,53 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; import { makeCapletController } from './caplet-controller.ts'; +import type { CapletControllerState } from './caplet-controller.ts'; import type { CapletManifest } from './types.ts'; -import type { NamespacedStorage } from '../storage/types.ts'; +import type { ControllerStorage } from '../storage/controller-storage.ts'; import type { ControllerConfig } from '../types.ts'; +/** + * Create a mock ControllerStorage for testing. + * Maintains in-memory state and tracks update calls. + * + * @param initialState - The initial state for the mock storage. + * @returns A mock ControllerStorage instance with update tracking. + */ +function createMockStorage( + initialState: CapletControllerState, +): ControllerStorage & { updateCalls: (() => void)[] } { + let currentState = { ...initialState }; + const updateCalls: (() => void)[] = []; + + return { + get state(): Readonly { + return harden({ ...currentState }); + }, + + async update( + producer: (draft: CapletControllerState) => void, + ): Promise { + // Create a mutable draft + const draft = JSON.parse( + JSON.stringify(currentState), + ) as CapletControllerState; + producer(draft); + currentState = draft; + updateCalls.push(() => producer(draft)); + }, + + async reload(): Promise { + // No-op for tests + }, + + updateCalls, + }; +} + +const emptyState: CapletControllerState = { + caplets: {}, +}; + describe('makeCapletController', () => { const mockLogger = { info: vi.fn(), @@ -14,15 +57,6 @@ describe('makeCapletController', () => { subLogger: vi.fn().mockReturnThis(), }; - const mockStorage: NamespacedStorage = { - get: vi.fn(), - set: vi.fn(), - delete: vi.fn(), - has: vi.fn(), - keys: vi.fn(), - clear: vi.fn(), - }; - const mockLaunchSubcluster = vi.fn(); const mockTerminateSubcluster = vi.fn(); @@ -30,12 +64,6 @@ describe('makeCapletController', () => { logger: mockLogger as unknown as ControllerConfig['logger'], }; - const deps = { - storage: mockStorage, - launchSubcluster: mockLaunchSubcluster, - terminateSubcluster: mockTerminateSubcluster, - }; - const validManifest: CapletManifest = { id: 'com.example.test', name: 'Test Caplet', @@ -46,8 +74,7 @@ describe('makeCapletController', () => { }; beforeEach(() => { - vi.mocked(mockStorage.has).mockResolvedValue(false); - vi.mocked(mockStorage.keys).mockResolvedValue([]); + vi.clearAllMocks(); vi.mocked(mockLaunchSubcluster).mockResolvedValue({ subclusterId: 'subcluster-123', }); @@ -55,7 +82,13 @@ describe('makeCapletController', () => { describe('install', () => { it('installs a caplet successfully', async () => { - const controller = makeCapletController(config, deps); + const mockStorage = createMockStorage(emptyState); + const controller = makeCapletController(config, { + storage: mockStorage, + launchSubcluster: mockLaunchSubcluster, + terminateSubcluster: mockTerminateSubcluster, + }); + const result = await controller.install(validManifest); expect(result).toStrictEqual({ @@ -65,7 +98,13 @@ describe('makeCapletController', () => { }); it('validates the manifest', async () => { - const controller = makeCapletController(config, deps); + const mockStorage = createMockStorage(emptyState); + const controller = makeCapletController(config, { + storage: mockStorage, + launchSubcluster: mockLaunchSubcluster, + terminateSubcluster: mockTerminateSubcluster, + }); + const invalidManifest = { id: 'invalid' } as CapletManifest; await expect(controller.install(invalidManifest)).rejects.toThrow( @@ -74,22 +113,35 @@ describe('makeCapletController', () => { }); it('throws if caplet already installed', async () => { - vi.mocked(mockStorage.get).mockImplementation(async (key: string) => { - if (key === 'com.example.test.manifest') { - return validManifest; - } - return undefined; + const stateWithCaplet: CapletControllerState = { + caplets: { + 'com.example.test': { + manifest: validManifest, + subclusterId: 'subcluster-123', + installedAt: 1000, + }, + }, + }; + const mockStorage = createMockStorage(stateWithCaplet); + const controller = makeCapletController(config, { + storage: mockStorage, + launchSubcluster: mockLaunchSubcluster, + terminateSubcluster: mockTerminateSubcluster, }); - const controller = makeCapletController(config, deps); - await expect(controller.install(validManifest)).rejects.toThrow( 'Caplet com.example.test is already installed', ); }); it('launches subcluster with correct config', async () => { - const controller = makeCapletController(config, deps); + const mockStorage = createMockStorage(emptyState); + const controller = makeCapletController(config, { + storage: mockStorage, + launchSubcluster: mockLaunchSubcluster, + terminateSubcluster: mockTerminateSubcluster, + }); + await controller.install(validManifest); expect(mockLaunchSubcluster).toHaveBeenCalledWith({ @@ -102,67 +154,61 @@ describe('makeCapletController', () => { }); }); - it('stores manifest, subclusterId, and installedAt', async () => { + it('stores caplet with manifest, subclusterId, and installedAt', async () => { vi.useFakeTimers(); vi.setSystemTime(new Date('2024-01-15T12:00:00Z')); - const controller = makeCapletController(config, deps); + const mockStorage = createMockStorage(emptyState); + const controller = makeCapletController(config, { + storage: mockStorage, + launchSubcluster: mockLaunchSubcluster, + terminateSubcluster: mockTerminateSubcluster, + }); + await controller.install(validManifest); - expect(mockStorage.set).toHaveBeenCalledWith( - 'com.example.test.manifest', - validManifest, - ); - expect(mockStorage.set).toHaveBeenCalledWith( - 'com.example.test.subclusterId', - 'subcluster-123', - ); - expect(mockStorage.set).toHaveBeenCalledWith( - 'com.example.test.installedAt', - Date.now(), - ); + const caplet = mockStorage.state.caplets['com.example.test']; + expect(caplet).toBeDefined(); + expect(caplet?.manifest).toStrictEqual(validManifest); + expect(caplet?.subclusterId).toBe('subcluster-123'); + expect(caplet?.installedAt).toBe(Date.now()); vi.useRealTimers(); }); - it('updates installed list', async () => { - vi.mocked(mockStorage.get).mockImplementation(async (key: string) => { - if (key === 'installed') { - return ['com.other.caplet']; - } - return undefined; + it('preserves existing caplets when installing', async () => { + const stateWithOtherCaplet: CapletControllerState = { + caplets: { + 'com.other.caplet': { + manifest: { ...validManifest, id: 'com.other.caplet' }, + subclusterId: 'subcluster-other', + installedAt: 500, + }, + }, + }; + const mockStorage = createMockStorage(stateWithOtherCaplet); + const controller = makeCapletController(config, { + storage: mockStorage, + launchSubcluster: mockLaunchSubcluster, + terminateSubcluster: mockTerminateSubcluster, }); - const controller = makeCapletController(config, deps); await controller.install(validManifest); - expect(mockStorage.set).toHaveBeenCalledWith('installed', [ + expect(Object.keys(mockStorage.state.caplets)).toStrictEqual([ 'com.other.caplet', 'com.example.test', ]); }); - it('does not duplicate caplet id in installed list', async () => { - vi.mocked(mockStorage.get).mockImplementation(async (key: string) => { - if (key === 'installed') { - return ['com.example.test']; - } - // Return undefined for manifest to allow install to proceed - return undefined; + it('logs installation progress', async () => { + const mockStorage = createMockStorage(emptyState); + const controller = makeCapletController(config, { + storage: mockStorage, + launchSubcluster: mockLaunchSubcluster, + terminateSubcluster: mockTerminateSubcluster, }); - const controller = makeCapletController(config, deps); - await controller.install(validManifest); - - // Should not add duplicate - expect(mockStorage.set).not.toHaveBeenCalledWith('installed', [ - 'com.example.test', - 'com.example.test', - ]); - }); - - it('logs installation progress', async () => { - const controller = makeCapletController(config, deps); await controller.install(validManifest); expect(mockLogger.info).toHaveBeenCalledWith( @@ -176,86 +222,108 @@ describe('makeCapletController', () => { describe('uninstall', () => { it('uninstalls a caplet successfully', async () => { - vi.mocked(mockStorage.get).mockImplementation(async (key: string) => { - if (key === 'com.example.test.subclusterId') { - return 'subcluster-123'; - } - if (key === 'installed') { - return ['com.example.test']; - } - return undefined; + const stateWithCaplet: CapletControllerState = { + caplets: { + 'com.example.test': { + manifest: validManifest, + subclusterId: 'subcluster-123', + installedAt: 1000, + }, + }, + }; + const mockStorage = createMockStorage(stateWithCaplet); + const controller = makeCapletController(config, { + storage: mockStorage, + launchSubcluster: mockLaunchSubcluster, + terminateSubcluster: mockTerminateSubcluster, }); - const controller = makeCapletController(config, deps); await controller.uninstall('com.example.test'); expect(mockTerminateSubcluster).toHaveBeenCalledWith('subcluster-123'); }); it('throws if caplet not found', async () => { - const controller = makeCapletController(config, deps); + const mockStorage = createMockStorage(emptyState); + const controller = makeCapletController(config, { + storage: mockStorage, + launchSubcluster: mockLaunchSubcluster, + terminateSubcluster: mockTerminateSubcluster, + }); await expect( controller.uninstall('com.example.notfound'), ).rejects.toThrow('Caplet com.example.notfound not found'); }); - it('removes all caplet data from storage', async () => { - vi.mocked(mockStorage.get).mockImplementation(async (key: string) => { - if (key === 'com.example.test.subclusterId') { - return 'subcluster-123'; - } - if (key === 'installed') { - return ['com.example.test']; - } - return undefined; + it('removes caplet from state', async () => { + const stateWithCaplet: CapletControllerState = { + caplets: { + 'com.example.test': { + manifest: validManifest, + subclusterId: 'subcluster-123', + installedAt: 1000, + }, + }, + }; + const mockStorage = createMockStorage(stateWithCaplet); + const controller = makeCapletController(config, { + storage: mockStorage, + launchSubcluster: mockLaunchSubcluster, + terminateSubcluster: mockTerminateSubcluster, }); - const controller = makeCapletController(config, deps); await controller.uninstall('com.example.test'); - expect(mockStorage.delete).toHaveBeenCalledWith( - 'com.example.test.manifest', - ); - expect(mockStorage.delete).toHaveBeenCalledWith( - 'com.example.test.subclusterId', - ); - expect(mockStorage.delete).toHaveBeenCalledWith( - 'com.example.test.installedAt', - ); + expect(mockStorage.state.caplets['com.example.test']).toBeUndefined(); }); - it('updates installed list', async () => { - vi.mocked(mockStorage.get).mockImplementation(async (key: string) => { - if (key === 'com.example.test.subclusterId') { - return 'subcluster-123'; - } - if (key === 'installed') { - return ['com.other.caplet', 'com.example.test']; - } - return undefined; + it('preserves other caplets when uninstalling', async () => { + const stateWithCaplets: CapletControllerState = { + caplets: { + 'com.other.caplet': { + manifest: { ...validManifest, id: 'com.other.caplet' }, + subclusterId: 'subcluster-other', + installedAt: 500, + }, + 'com.example.test': { + manifest: validManifest, + subclusterId: 'subcluster-123', + installedAt: 1000, + }, + }, + }; + const mockStorage = createMockStorage(stateWithCaplets); + const controller = makeCapletController(config, { + storage: mockStorage, + launchSubcluster: mockLaunchSubcluster, + terminateSubcluster: mockTerminateSubcluster, }); - const controller = makeCapletController(config, deps); await controller.uninstall('com.example.test'); - expect(mockStorage.set).toHaveBeenCalledWith('installed', [ + expect(Object.keys(mockStorage.state.caplets)).toStrictEqual([ 'com.other.caplet', ]); }); it('logs uninstallation progress', async () => { - vi.mocked(mockStorage.get).mockImplementation(async (key: string) => { - if (key === 'com.example.test.subclusterId') { - return 'subcluster-123'; - } - if (key === 'installed') { - return ['com.example.test']; - } - return undefined; + const stateWithCaplet: CapletControllerState = { + caplets: { + 'com.example.test': { + manifest: validManifest, + subclusterId: 'subcluster-123', + installedAt: 1000, + }, + }, + }; + const mockStorage = createMockStorage(stateWithCaplet); + const controller = makeCapletController(config, { + storage: mockStorage, + launchSubcluster: mockLaunchSubcluster, + terminateSubcluster: mockTerminateSubcluster, }); - const controller = makeCapletController(config, deps); await controller.uninstall('com.example.test'); expect(mockLogger.info).toHaveBeenCalledWith( @@ -269,7 +337,13 @@ describe('makeCapletController', () => { describe('list', () => { it('returns empty array when no caplets installed', async () => { - const controller = makeCapletController(config, deps); + const mockStorage = createMockStorage(emptyState); + const controller = makeCapletController(config, { + storage: mockStorage, + launchSubcluster: mockLaunchSubcluster, + terminateSubcluster: mockTerminateSubcluster, + }); + const result = await controller.list(); expect(result).toStrictEqual([]); @@ -281,90 +355,61 @@ describe('makeCapletController', () => { id: 'com.example.test2', name: 'Test Caplet 2', }; - - vi.mocked(mockStorage.get).mockImplementation(async (key: string) => { - if (key === 'installed') { - return ['com.example.test', 'com.example.test2']; - } - if (key === 'com.example.test.manifest') { - return validManifest; - } - if (key === 'com.example.test.subclusterId') { - return 'subcluster-1'; - } - if (key === 'com.example.test.installedAt') { - return 1000; - } - if (key === 'com.example.test2.manifest') { - return manifest2; - } - if (key === 'com.example.test2.subclusterId') { - return 'subcluster-2'; - } - if (key === 'com.example.test2.installedAt') { - return 2000; - } - return undefined; + const stateWithCaplets: CapletControllerState = { + caplets: { + 'com.example.test': { + manifest: validManifest, + subclusterId: 'subcluster-1', + installedAt: 1000, + }, + 'com.example.test2': { + manifest: manifest2, + subclusterId: 'subcluster-2', + installedAt: 2000, + }, + }, + }; + const mockStorage = createMockStorage(stateWithCaplets); + const controller = makeCapletController(config, { + storage: mockStorage, + launchSubcluster: mockLaunchSubcluster, + terminateSubcluster: mockTerminateSubcluster, }); - const controller = makeCapletController(config, deps); const result = await controller.list(); expect(result).toHaveLength(2); - expect(result[0]).toStrictEqual({ + expect(result).toContainEqual({ manifest: validManifest, subclusterId: 'subcluster-1', installedAt: 1000, }); - expect(result[1]).toStrictEqual({ + expect(result).toContainEqual({ manifest: manifest2, subclusterId: 'subcluster-2', installedAt: 2000, }); }); - - it('skips caplets with missing data', async () => { - vi.mocked(mockStorage.get).mockImplementation(async (key: string) => { - if (key === 'installed') { - return ['com.example.test', 'com.example.missing']; - } - if (key === 'com.example.test.manifest') { - return validManifest; - } - if (key === 'com.example.test.subclusterId') { - return 'subcluster-1'; - } - if (key === 'com.example.test.installedAt') { - return 1000; - } - // com.example.missing has no data - return undefined; - }); - - const controller = makeCapletController(config, deps); - const result = await controller.list(); - - expect(result).toHaveLength(1); - expect(result[0]?.manifest.id).toBe('com.example.test'); - }); }); describe('get', () => { it('returns caplet if exists', async () => { - vi.mocked(mockStorage.get).mockImplementation(async (key: string) => { - if (key === 'com.example.test.manifest') { - return validManifest; - } - if (key === 'com.example.test.subclusterId') { - return 'subcluster-123'; - } - if (key === 'com.example.test.installedAt') { - return 1705320000000; - } - return undefined; + const stateWithCaplet: CapletControllerState = { + caplets: { + 'com.example.test': { + manifest: validManifest, + subclusterId: 'subcluster-123', + installedAt: 1705320000000, + }, + }, + }; + const mockStorage = createMockStorage(stateWithCaplet); + const controller = makeCapletController(config, { + storage: mockStorage, + launchSubcluster: mockLaunchSubcluster, + terminateSubcluster: mockTerminateSubcluster, }); - const controller = makeCapletController(config, deps); const result = await controller.get('com.example.test'); expect(result).toStrictEqual({ @@ -375,50 +420,37 @@ describe('makeCapletController', () => { }); it('returns undefined if caplet not found', async () => { - const controller = makeCapletController(config, deps); - const result = await controller.get('com.example.notfound'); - - expect(result).toBeUndefined(); - }); - - it('returns undefined and logs warning if storage data corrupted', async () => { - vi.mocked(mockStorage.get).mockImplementation(async (key: string) => { - if (key === 'com.example.test.manifest') { - return validManifest; - } - // Missing subclusterId and installedAt - return undefined; + const mockStorage = createMockStorage(emptyState); + const controller = makeCapletController(config, { + storage: mockStorage, + launchSubcluster: mockLaunchSubcluster, + terminateSubcluster: mockTerminateSubcluster, }); - const controller = makeCapletController(config, deps); - const result = await controller.get('com.example.test'); + const result = await controller.get('com.example.notfound'); expect(result).toBeUndefined(); - expect(mockLogger.warn).toHaveBeenCalledWith( - 'Caplet com.example.test has corrupted storage data', - ); }); }); describe('getByService', () => { it('returns caplet providing the service', async () => { - vi.mocked(mockStorage.get).mockImplementation(async (key: string) => { - if (key === 'installed') { - return ['com.example.test']; - } - if (key === 'com.example.test.manifest') { - return validManifest; // providedServices: ['signer'] - } - if (key === 'com.example.test.subclusterId') { - return 'subcluster-123'; - } - if (key === 'com.example.test.installedAt') { - return 1000; - } - return undefined; + const stateWithCaplet: CapletControllerState = { + caplets: { + 'com.example.test': { + manifest: validManifest, + subclusterId: 'subcluster-123', + installedAt: 1000, + }, + }, + }; + const mockStorage = createMockStorage(stateWithCaplet); + const controller = makeCapletController(config, { + storage: mockStorage, + launchSubcluster: mockLaunchSubcluster, + terminateSubcluster: mockTerminateSubcluster, }); - const controller = makeCapletController(config, deps); const result = await controller.getByService('signer'); expect(result).toBeDefined(); @@ -426,66 +458,59 @@ describe('makeCapletController', () => { }); it('returns undefined if no caplet provides the service', async () => { - vi.mocked(mockStorage.get).mockImplementation(async (key: string) => { - if (key === 'installed') { - return ['com.example.test']; - } - if (key === 'com.example.test.manifest') { - return validManifest; - } - if (key === 'com.example.test.subclusterId') { - return 'subcluster-123'; - } - if (key === 'com.example.test.installedAt') { - return 1000; - } - return undefined; + const stateWithCaplet: CapletControllerState = { + caplets: { + 'com.example.test': { + manifest: validManifest, + subclusterId: 'subcluster-123', + installedAt: 1000, + }, + }, + }; + const mockStorage = createMockStorage(stateWithCaplet); + const controller = makeCapletController(config, { + storage: mockStorage, + launchSubcluster: mockLaunchSubcluster, + terminateSubcluster: mockTerminateSubcluster, }); - const controller = makeCapletController(config, deps); const result = await controller.getByService('unknown-service'); expect(result).toBeUndefined(); }); - it('returns first matching caplet when multiple provide the service', async () => { + it('returns a matching caplet when multiple provide the service', async () => { const manifest2: CapletManifest = { ...validManifest, id: 'com.example.test2', name: 'Test Caplet 2', providedServices: ['signer', 'verifier'], }; - - vi.mocked(mockStorage.get).mockImplementation(async (key: string) => { - if (key === 'installed') { - return ['com.example.test', 'com.example.test2']; - } - if (key === 'com.example.test.manifest') { - return validManifest; - } - if (key === 'com.example.test.subclusterId') { - return 'subcluster-1'; - } - if (key === 'com.example.test.installedAt') { - return 1000; - } - if (key === 'com.example.test2.manifest') { - return manifest2; - } - if (key === 'com.example.test2.subclusterId') { - return 'subcluster-2'; - } - if (key === 'com.example.test2.installedAt') { - return 2000; - } - return undefined; + const stateWithCaplets: CapletControllerState = { + caplets: { + 'com.example.test': { + manifest: validManifest, + subclusterId: 'subcluster-1', + installedAt: 1000, + }, + 'com.example.test2': { + manifest: manifest2, + subclusterId: 'subcluster-2', + installedAt: 2000, + }, + }, + }; + const mockStorage = createMockStorage(stateWithCaplets); + const controller = makeCapletController(config, { + storage: mockStorage, + launchSubcluster: mockLaunchSubcluster, + terminateSubcluster: mockTerminateSubcluster, }); - const controller = makeCapletController(config, deps); const result = await controller.getByService('signer'); - // Returns first match - expect(result?.manifest.id).toBe('com.example.test'); + // Returns a match (object key order is not guaranteed) + expect(result?.manifest.providedServices).toContain('signer'); }); }); }); diff --git a/packages/omnium-gatherum/src/controllers/caplet/caplet-controller.ts b/packages/omnium-gatherum/src/controllers/caplet/caplet-controller.ts index 3a4ed0ed0..8c7b8a700 100644 --- a/packages/omnium-gatherum/src/controllers/caplet/caplet-controller.ts +++ b/packages/omnium-gatherum/src/controllers/caplet/caplet-controller.ts @@ -9,49 +9,18 @@ import type { LaunchResult, } from './types.ts'; import { isCapletManifest } from './types.ts'; -import type { NamespacedStorage } from '../storage/types.ts'; +import type { ControllerStorage } from '../storage/controller-storage.ts'; import type { ControllerConfig } from '../types.ts'; /** - * Storage keys used by the CapletController within its namespace. + * Caplet controller persistent state. + * This is the shape of the state managed by the CapletController + * through the ControllerStorage abstraction. */ -const STORAGE_KEYS = { - /** List of installed caplet IDs */ - INSTALLED_LIST: 'installed', - /** Suffix for manifest storage: `${capletId}.manifest` */ - MANIFEST_SUFFIX: '.manifest', - /** Suffix for subclusterId storage: `${capletId}.subclusterId` */ - SUBCLUSTER_SUFFIX: '.subclusterId', - /** Suffix for installedAt storage: `${capletId}.installedAt` */ - INSTALLED_AT_SUFFIX: '.installedAt', -} as const; - -/** - * Generate storage key for a caplet's manifest. - * - * @param capletId - The caplet ID. - * @returns The storage key. - */ -const manifestKey = (capletId: CapletId): string => - `${capletId}${STORAGE_KEYS.MANIFEST_SUFFIX}`; - -/** - * Generate storage key for a caplet's subclusterId. - * - * @param capletId - The caplet ID. - * @returns The storage key. - */ -const subclusterKey = (capletId: CapletId): string => - `${capletId}${STORAGE_KEYS.SUBCLUSTER_SUFFIX}`; - -/** - * Generate storage key for a caplet's installedAt timestamp. - * - * @param capletId - The caplet ID. - * @returns The storage key. - */ -const installedAtKey = (capletId: CapletId): string => - `${capletId}${STORAGE_KEYS.INSTALLED_AT_SUFFIX}`; +export type CapletControllerState = { + /** Installed caplets keyed by caplet ID */ + caplets: Record; +}; /** * Methods exposed by the CapletController. @@ -105,8 +74,8 @@ export type CapletControllerMethods = { * These are attenuated - only the methods needed are provided. */ export type CapletControllerDeps = { - /** Namespaced storage for caplet data */ - storage: NamespacedStorage; + /** State storage for caplet data */ + storage: ControllerStorage; /** Launch a subcluster for a caplet */ launchSubcluster: (config: ClusterConfig) => Promise; /** Terminate a caplet's subcluster */ @@ -133,73 +102,22 @@ export function makeCapletController( const { storage, launchSubcluster, terminateSubcluster } = deps; /** - * Get the list of installed caplet IDs. - * - * @returns Array of installed caplet IDs. - */ - const getInstalledIds = async (): Promise => { - const ids = await storage.get(STORAGE_KEYS.INSTALLED_LIST); - return ids ?? []; - }; - - /** - * Update the list of installed caplet IDs. - * - * @param ids - The list of caplet IDs to store. - */ - const setInstalledIds = async (ids: CapletId[]): Promise => { - await storage.set(STORAGE_KEYS.INSTALLED_LIST, ids); - }; - - /** - * Internal get implementation (to avoid `this` binding issues in exo). + * Get an installed caplet by ID (synchronous - reads from in-memory state). * * @param capletId - The caplet ID to retrieve. * @returns The installed caplet or undefined if not found. */ - const getCaplet = async ( - capletId: CapletId, - ): Promise => { - const manifest = await storage.get(manifestKey(capletId)); - if (manifest === undefined) { - return undefined; - } - - const [subclusterId, installedAt] = await Promise.all([ - storage.get(subclusterKey(capletId)), - storage.get(installedAtKey(capletId)), - ]); - - if (subclusterId === undefined || installedAt === undefined) { - // Corrupted data - manifest exists but other fields don't - logger.warn(`Caplet ${capletId} has corrupted storage data`); - return undefined; - } - - return { - manifest, - subclusterId, - installedAt, - }; + const getCaplet = (capletId: CapletId): InstalledCaplet | undefined => { + return storage.state.caplets[capletId]; }; /** - * Internal list implementation (to avoid `this` binding issues in exo). + * Get all installed caplets (synchronous - reads from in-memory state). * * @returns Array of all installed caplets. */ - const listCaplets = async (): Promise => { - const installedIds = await getInstalledIds(); - const caplets: InstalledCaplet[] = []; - - for (const id of installedIds) { - const caplet = await getCaplet(id); - if (caplet !== undefined) { - caplets.push(caplet); - } - } - - return caplets; + const listCaplets = (): InstalledCaplet[] => { + return Object.values(storage.state.caplets); }; return makeDefaultExo('CapletController', { @@ -216,8 +134,7 @@ export function makeCapletController( } // Check if already installed - const existing = await storage.get(manifestKey(id)); - if (existing !== undefined) { + if (storage.state.caplets[id] !== undefined) { throw new Error(`Caplet ${id} is already installed`); } @@ -235,18 +152,13 @@ export function makeCapletController( const { subclusterId } = await launchSubcluster(clusterConfig); // Store caplet data - const now = Date.now(); - await Promise.all([ - storage.set(manifestKey(id), manifest), - storage.set(subclusterKey(id), subclusterId), - storage.set(installedAtKey(id), now), - ]); - - // Update installed list - const installedIds = await getInstalledIds(); - if (!installedIds.includes(id)) { - await setInstalledIds([...installedIds, id]); - } + await storage.update((draft) => { + draft.caplets[id] = { + manifest, + subclusterId, + installedAt: Date.now(), + }; + }); logger.info(`Caplet ${id} installed with subcluster ${subclusterId}`); return { capletId: id, subclusterId }; @@ -255,24 +167,18 @@ export function makeCapletController( async uninstall(capletId: CapletId): Promise { logger.info(`Uninstalling caplet: ${capletId}`); - const subclusterId = await storage.get(subclusterKey(capletId)); - if (subclusterId === undefined) { + const caplet = storage.state.caplets[capletId]; + if (caplet === undefined) { throw new Error(`Caplet ${capletId} not found`); } // Terminate the subcluster - await terminateSubcluster(subclusterId); + await terminateSubcluster(caplet.subclusterId); // Remove from storage - await Promise.all([ - storage.delete(manifestKey(capletId)), - storage.delete(subclusterKey(capletId)), - storage.delete(installedAtKey(capletId)), - ]); - - // Update installed list - const installedIds = await getInstalledIds(); - await setInstalledIds(installedIds.filter((id) => id !== capletId)); + await storage.update((draft) => { + delete draft.caplets[capletId]; + }); logger.info(`Caplet ${capletId} uninstalled`); }, @@ -288,7 +194,7 @@ export function makeCapletController( async getByService( serviceName: string, ): Promise { - const caplets = await listCaplets(); + const caplets = listCaplets(); return caplets.find((caplet: InstalledCaplet) => caplet.manifest.providedServices.includes(serviceName), ); diff --git a/packages/omnium-gatherum/src/controllers/caplet/index.ts b/packages/omnium-gatherum/src/controllers/caplet/index.ts index e2b30889c..e0cb3f5cf 100644 --- a/packages/omnium-gatherum/src/controllers/caplet/index.ts +++ b/packages/omnium-gatherum/src/controllers/caplet/index.ts @@ -18,5 +18,6 @@ export { export type { CapletControllerMethods, CapletControllerDeps, + CapletControllerState, } from './caplet-controller.ts'; export { makeCapletController } from './caplet-controller.ts'; diff --git a/packages/omnium-gatherum/src/controllers/facet.ts b/packages/omnium-gatherum/src/controllers/facet.ts index 95e8a1338..2ec6dc269 100644 --- a/packages/omnium-gatherum/src/controllers/facet.ts +++ b/packages/omnium-gatherum/src/controllers/facet.ts @@ -1,13 +1,14 @@ -import type { Methods, MethodGuard } from '@endo/exo'; +import type { Methods } from '@endo/exo'; import { makeDefaultExo } from '@metamask/kernel-utils/exo'; -// RemotableMethodName from @endo/pass-style is string | symbol -type MethodKeys = Extract< - { - [Key in keyof Source]: Source[Key] extends CallableFunction ? Key : never; - }[keyof Source], - keyof MethodGuard ->; +/** + * Extract keys from Source that are callable functions. + * Filters to string | symbol to match RemotableMethodName from @endo/pass-style. + */ +type MethodKeys = { + [Key in keyof Source]: Source[Key] extends CallableFunction ? Key : never; +}[keyof Source] & + (string | symbol); type BoundMethod = Func extends CallableFunction ? OmitThisParameter diff --git a/packages/omnium-gatherum/src/controllers/index.ts b/packages/omnium-gatherum/src/controllers/index.ts index 25b792d4b..22abb8f00 100644 --- a/packages/omnium-gatherum/src/controllers/index.ts +++ b/packages/omnium-gatherum/src/controllers/index.ts @@ -3,10 +3,16 @@ export type { ControllerConfig, FacetOf } from './types.ts'; export { makeFacet } from './facet.ts'; // Storage -export type { NamespacedStorage, StorageAdapter } from './storage/index.ts'; +export type { + NamespacedStorage, + StorageAdapter, + ControllerStorage, + ControllerStorageConfig, +} from './storage/index.ts'; export { makeChromeStorageAdapter, makeNamespacedStorage, + makeControllerStorage, } from './storage/index.ts'; // Caplet @@ -17,6 +23,7 @@ export type { InstalledCaplet, InstallResult, LaunchResult, + CapletControllerState, CapletControllerMethods, CapletControllerDeps, } from './caplet/index.ts'; diff --git a/packages/omnium-gatherum/src/controllers/storage/controller-storage.test.ts b/packages/omnium-gatherum/src/controllers/storage/controller-storage.test.ts new file mode 100644 index 000000000..c01093441 --- /dev/null +++ b/packages/omnium-gatherum/src/controllers/storage/controller-storage.test.ts @@ -0,0 +1,336 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; + +import { makeControllerStorage } from './controller-storage.ts'; +import type { StorageAdapter } from './types.ts'; + +type TestState = { + installed: string[]; + manifests: Record; + count: number; +}; + +describe('makeControllerStorage', () => { + const mockAdapter: StorageAdapter = { + get: vi.fn(), + set: vi.fn(), + delete: vi.fn(), + keys: vi.fn(), + }; + + const defaultState: TestState = { + installed: [], + manifests: {}, + count: 0, + }; + + beforeEach(() => { + vi.mocked(mockAdapter.get).mockResolvedValue(undefined); + vi.mocked(mockAdapter.set).mockResolvedValue(undefined); + vi.mocked(mockAdapter.delete).mockResolvedValue(undefined); + vi.mocked(mockAdapter.keys).mockResolvedValue([]); + }); + + describe('initialization', () => { + it('loads existing state from storage on creation', async () => { + vi.mocked(mockAdapter.keys).mockResolvedValue([ + 'test.installed', + 'test.manifests', + ]); + vi.mocked(mockAdapter.get).mockImplementation(async (key: string) => { + if (key === 'test.installed') { + return ['app1']; + } + if (key === 'test.manifests') { + return { app1: { name: 'App 1' } }; + } + return undefined; + }); + + const storage = await makeControllerStorage({ + namespace: 'test', + adapter: mockAdapter, + defaultState, + }); + + expect(storage.state.installed).toStrictEqual(['app1']); + expect(storage.state.manifests).toStrictEqual({ + app1: { name: 'App 1' }, + }); + }); + + it('uses defaults for missing keys', async () => { + vi.mocked(mockAdapter.keys).mockResolvedValue(['test.installed']); + vi.mocked(mockAdapter.get).mockImplementation(async (key: string) => { + if (key === 'test.installed') { + return ['existing']; + } + return undefined; + }); + + const storage = await makeControllerStorage({ + namespace: 'test', + adapter: mockAdapter, + defaultState: { + installed: [] as string[], + manifests: {}, + metadata: { version: 1 }, + }, + }); + + expect(storage.state.installed).toStrictEqual(['existing']); + expect(storage.state.manifests).toStrictEqual({}); + expect(storage.state.metadata).toStrictEqual({ version: 1 }); + }); + + it('uses all defaults when storage is empty', async () => { + vi.mocked(mockAdapter.keys).mockResolvedValue([]); + + const storage = await makeControllerStorage({ + namespace: 'test', + adapter: mockAdapter, + defaultState, + }); + + expect(storage.state.installed).toStrictEqual([]); + expect(storage.state.manifests).toStrictEqual({}); + expect(storage.state.count).toBe(0); + }); + + it('returns hardened state copy', async () => { + const storage = await makeControllerStorage({ + namespace: 'test', + adapter: mockAdapter, + defaultState: { items: ['original'] as string[] }, + }); + + // Get a reference to the state + const state1 = storage.state; + + // Modifications to the returned state should not affect the internal state + // (In SES environment, this would throw; in tests, we verify isolation) + try { + (state1 as { items: string[] }).items.push('modified'); + } catch { + // Expected in SES environment + } + + // Get a fresh state - it should still have the original value + const state2 = storage.state; + expect(state2.items).toStrictEqual(['original']); + }); + }); + + describe('state access', () => { + it('provides readonly access to current state', async () => { + vi.mocked(mockAdapter.keys).mockResolvedValue(['ns.count']); + vi.mocked(mockAdapter.get).mockResolvedValue(42); + + const storage = await makeControllerStorage({ + namespace: 'ns', + adapter: mockAdapter, + defaultState: { count: 0 }, + }); + + expect(storage.state.count).toBe(42); + }); + }); + + describe('update', () => { + it('persists only modified top-level keys', async () => { + const storage = await makeControllerStorage({ + namespace: 'test', + adapter: mockAdapter, + defaultState, + }); + + await storage.update((draft) => { + draft.installed.push('new-app'); + // manifests and count not modified + }); + + expect(mockAdapter.set).toHaveBeenCalledTimes(1); + expect(mockAdapter.set).toHaveBeenCalledWith('test.installed', [ + 'new-app', + ]); + }); + + it('updates in-memory state after persistence', async () => { + const storage = await makeControllerStorage({ + namespace: 'test', + adapter: mockAdapter, + defaultState, + }); + + await storage.update((draft) => { + draft.installed.push('item1'); + }); + + expect(storage.state.installed).toStrictEqual(['item1']); + }); + + it('does not persist when no changes made', async () => { + const storage = await makeControllerStorage({ + namespace: 'test', + adapter: mockAdapter, + defaultState, + }); + + await storage.update((draft) => { + // No actual changes + draft.count = 0; + }); + + expect(mockAdapter.set).not.toHaveBeenCalled(); + }); + + it('persists multiple modified keys', async () => { + const storage = await makeControllerStorage({ + namespace: 'test', + adapter: mockAdapter, + defaultState: { a: 1, b: 2, c: 3 }, + }); + + await storage.update((draft) => { + draft.a = 10; + draft.c = 30; + }); + + expect(mockAdapter.set).toHaveBeenCalledTimes(2); + expect(mockAdapter.set).toHaveBeenCalledWith('test.a', 10); + expect(mockAdapter.set).toHaveBeenCalledWith('test.c', 30); + }); + + it('does not update state if persistence fails', async () => { + vi.mocked(mockAdapter.set).mockRejectedValue(new Error('Storage error')); + + const storage = await makeControllerStorage({ + namespace: 'test', + adapter: mockAdapter, + defaultState, + }); + + await expect( + storage.update((draft) => { + draft.count = 100; + }), + ).rejects.toThrow('Storage error'); + + // State should remain unchanged + expect(storage.state.count).toBe(0); + }); + + it('handles nested object modifications', async () => { + const storage = await makeControllerStorage({ + namespace: 'test', + adapter: mockAdapter, + defaultState, + }); + + await storage.update((draft) => { + draft.manifests['new-app'] = { name: 'New App' }; + }); + + expect(mockAdapter.set).toHaveBeenCalledWith('test.manifests', { + 'new-app': { name: 'New App' }, + }); + }); + + it('handles array operations', async () => { + vi.mocked(mockAdapter.keys).mockResolvedValue(['test.installed']); + vi.mocked(mockAdapter.get).mockResolvedValue(['app1', 'app2']); + + const storage = await makeControllerStorage({ + namespace: 'test', + adapter: mockAdapter, + defaultState, + }); + + await storage.update((draft) => { + draft.installed = draft.installed.filter((id) => id !== 'app1'); + }); + + expect(mockAdapter.set).toHaveBeenCalledWith('test.installed', ['app2']); + }); + + it('handles delete operations on nested objects', async () => { + vi.mocked(mockAdapter.keys).mockResolvedValue(['test.manifests']); + vi.mocked(mockAdapter.get).mockResolvedValue({ + app1: { name: 'App 1' }, + app2: { name: 'App 2' }, + }); + + const storage = await makeControllerStorage({ + namespace: 'test', + adapter: mockAdapter, + defaultState, + }); + + await storage.update((draft) => { + delete draft.manifests.app1; + }); + + expect(mockAdapter.set).toHaveBeenCalledWith('test.manifests', { + app2: { name: 'App 2' }, + }); + }); + }); + + describe('reload', () => { + it('reloads state from storage', async () => { + vi.mocked(mockAdapter.keys).mockResolvedValue([]); + + const storage = await makeControllerStorage({ + namespace: 'test', + adapter: mockAdapter, + defaultState, + }); + + expect(storage.state.count).toBe(0); + + // Simulate external storage update + vi.mocked(mockAdapter.keys).mockResolvedValue(['test.count']); + vi.mocked(mockAdapter.get).mockResolvedValue(999); + + await storage.reload(); + + expect(storage.state.count).toBe(999); + }); + + it('merges with defaults after reload', async () => { + vi.mocked(mockAdapter.keys).mockResolvedValue(['test.count']); + vi.mocked(mockAdapter.get).mockResolvedValue(42); + + const storage = await makeControllerStorage({ + namespace: 'test', + adapter: mockAdapter, + defaultState, + }); + + // Reload - count from storage, others from defaults + await storage.reload(); + + expect(storage.state.count).toBe(42); + expect(storage.state.installed).toStrictEqual([]); + expect(storage.state.manifests).toStrictEqual({}); + }); + }); + + describe('namespace isolation', () => { + it('uses different prefixes for different namespaces', async () => { + await makeControllerStorage({ + namespace: 'caplet', + adapter: mockAdapter, + defaultState: { value: 1 }, + }); + + await makeControllerStorage({ + namespace: 'service', + adapter: mockAdapter, + defaultState: { value: 2 }, + }); + + expect(mockAdapter.keys).toHaveBeenCalledWith('caplet.'); + expect(mockAdapter.keys).toHaveBeenCalledWith('service.'); + }); + }); +}); diff --git a/packages/omnium-gatherum/src/controllers/storage/controller-storage.ts b/packages/omnium-gatherum/src/controllers/storage/controller-storage.ts new file mode 100644 index 000000000..22ef617f7 --- /dev/null +++ b/packages/omnium-gatherum/src/controllers/storage/controller-storage.ts @@ -0,0 +1,224 @@ +import type { Json } from '@metamask/utils'; +import { enablePatches, produce } from 'immer'; +import type { Patch } from 'immer'; + +import type { StorageAdapter } from './types.ts'; + +// Enable immer patches globally (called once at module load) +enablePatches(); + +// TODO: Add migration utility for converting from per-key storage format +// (e.g., caplet.{id}.manifest) to consolidated state format (caplet.manifests) +// when there is deployed data to migrate. + +/** + * Configuration for creating a ControllerStorage instance. + */ +export type ControllerStorageConfig> = { + /** The namespace prefix for storage keys (e.g., 'caplet') */ + namespace: string; + /** The underlying storage adapter */ + adapter: StorageAdapter; + /** Default state values - used for initialization and type inference */ + defaultState: State; +}; + +/** + * ControllerStorage provides a simplified state management interface for controllers. + * + * Features: + * - Flat top-level key mapping: `state.foo` maps to `{namespace}.foo` in storage + * - Immer-based updates with automatic change detection + * - Only modified top-level keys are persisted + * - Eager loading on initialization + * + * @template State - The state object type (must have Json-serializable values) + */ +export type ControllerStorage> = { + /** + * Current state (readonly, hardened). + * Access individual properties: `storage.state.installed` + */ + readonly state: Readonly; + + /** + * Update state using an immer producer function. + * Only modified top-level keys will be persisted to storage. + * + * @param producer - Function that mutates a draft of the state + * @returns Promise that resolves when changes are persisted + * @throws If storage persistence fails (state remains unchanged) + * + * @example + * ```typescript + * await storage.update(draft => { + * draft.installed.push('com.example.app'); + * draft.manifests['com.example.app'] = manifest; + * }); + * ``` + */ + update: (producer: (draft: State) => void) => Promise; + + /** + * Force reload state from storage. + * Useful for syncing after external storage changes. + */ + reload: () => Promise; +}; + +/** + * Create a ControllerStorage instance for a controller. + * + * This factory function: + * 1. Loads existing state from storage for the namespace + * 2. Merges with defaults (storage values take precedence) + * 3. Returns a hardened ControllerStorage interface + * + * @param config - Configuration including namespace, adapter, and default state. + * @returns Promise resolving to a hardened ControllerStorage instance. + * + * @example + * ```typescript + * const capletState = await makeControllerStorage({ + * namespace: 'caplet', + * adapter: storageAdapter, + * defaultState: { installed: [], manifests: {} } + * }); + * + * // Read state + * console.log(capletState.state.installed); + * + * // Update state + * await capletState.update(draft => { + * draft.installed.push('com.example.app'); + * }); + * ``` + */ +export async function makeControllerStorage>( + config: ControllerStorageConfig, +): Promise> { + const { namespace, adapter, defaultState } = config; + const prefix = `${namespace}.`; + + /** + * Build a storage key from a state property name. + * + * @param stateKey - The state property name. + * @returns The namespaced storage key. + */ + const buildKey = (stateKey: string): string => `${prefix}${stateKey}`; + + /** + * Strip namespace prefix from a storage key. + * + * @param fullKey - The full namespaced storage key. + * @returns The state property name without prefix. + */ + const stripPrefix = (fullKey: string): string => fullKey.slice(prefix.length); + + /** + * Load all state from storage, merging with defaults. + * Storage values take precedence over defaults. + * + * @returns The merged state object. + */ + const loadState = async (): Promise => { + const allKeys = await adapter.keys(prefix); + + // Start with a copy of defaults + const state = { ...defaultState }; + + // Load and merge values from storage + await Promise.all( + allKeys.map(async (fullKey) => { + const key = stripPrefix(fullKey) as keyof State; + const value = await adapter.get(fullKey); + if (value !== undefined) { + state[key] = value as State[keyof State]; + } + }), + ); + + return produce({}, (draft) => { + Object.assign(draft, state); + }) as State; + }; + + /** + * Persist specific keys to storage. + * + * @param stateToSave - The state object containing values to persist. + * @param keys - Set of top-level keys to persist. + */ + const persistKeys = async ( + stateToSave: State, + keys: Set, + ): Promise => { + await Promise.all( + Array.from(keys).map(async (key) => { + const storageKey = buildKey(key); + const value = stateToSave[key as keyof State]; + await adapter.set(storageKey, value as Json); + }), + ); + }; + + /** + * Extract top-level keys that were modified from immer patches. + * + * @param patches - Array of immer patches describing changes. + * @returns Set of modified top-level keys. + */ + const getModifiedKeys = (patches: Patch[]): Set => { + const keys = new Set(); + for (const patch of patches) { + // The first element of path is always the top-level key + if (patch.path.length > 0) { + keys.add(String(patch.path[0])); + } + } + return keys; + }; + + // Load initial state + let currentState = await loadState(); + + const storage: ControllerStorage = { + get state(): Readonly { + return currentState; + }, + + async update(producer: (draft: State) => void): Promise { + // Capture state before async operations to avoid race conditions + const stateSnapshot = currentState; + + // Use immer's produce with patches callback to track changes + let patches: Patch[] = []; + const nextState = produce(stateSnapshot, producer, (patchList) => { + patches = patchList; + }); + + // No changes - nothing to do + if (patches.length === 0) { + return; + } + + // Determine which top-level keys changed + const modifiedKeys = getModifiedKeys(patches); + + // Persist only the modified keys + await persistKeys(nextState, modifiedKeys); + + // Update in-memory state only after successful persistence + // eslint-disable-next-line require-atomic-updates -- Last-write-wins is intentional + currentState = nextState; + }, + + async reload(): Promise { + currentState = await loadState(); + }, + }; + + return harden(storage); +} +harden(makeControllerStorage); diff --git a/packages/omnium-gatherum/src/controllers/storage/index.ts b/packages/omnium-gatherum/src/controllers/storage/index.ts index 5d9628d33..9a88b8acf 100644 --- a/packages/omnium-gatherum/src/controllers/storage/index.ts +++ b/packages/omnium-gatherum/src/controllers/storage/index.ts @@ -1,3 +1,8 @@ export type { NamespacedStorage, StorageAdapter } from './types.ts'; +export type { + ControllerStorage, + ControllerStorageConfig, +} from './controller-storage.ts'; export { makeChromeStorageAdapter } from './chrome-storage.ts'; export { makeNamespacedStorage } from './namespaced-storage.ts'; +export { makeControllerStorage } from './controller-storage.ts'; diff --git a/yarn.lock b/yarn.lock index cc6e6bc65..c8e29036d 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3956,6 +3956,7 @@ __metadata: eslint-plugin-n: "npm:^17.17.0" eslint-plugin-prettier: "npm:^5.2.6" eslint-plugin-promise: "npm:^7.2.1" + immer: "npm:^10.1.1" jsdom: "npm:^27.4.0" playwright: "npm:^1.54.2" prettier: "npm:^3.5.3" @@ -9985,6 +9986,13 @@ __metadata: languageName: node linkType: hard +"immer@npm:^10.1.1": + version: 10.2.0 + resolution: "immer@npm:10.2.0" + checksum: 10/d73e218c8f8ffbb39f9290dfafa478b94af73403dcf26b5672eef35233bb30f09ffe231f8a78a6c9cb442968510edd89e851776ec90a5ddfa82cee6db6b35137 + languageName: node + linkType: hard + "immer@npm:^9.0.6": version: 9.0.21 resolution: "immer@npm:9.0.21" From 0de9fe17a4576e4ff57ab03c3990f74229c31637 Mon Sep 17 00:00:00 2001 From: Erik Marks <25517051+rekmarks@users.noreply.github.com> Date: Fri, 9 Jan 2026 12:22:52 -0800 Subject: [PATCH 3/7] refactor(omnium): Add abstract Controller base class - Add abstract Controller class with state management via ControllerStorage - Convert CapletController to extend Controller base class - Use makeFacet() pattern for returning hardened exo methods - Add base-controller tests (12 tests) - Add semver deep import type declaration - Add storage permission to manifest.json Co-Authored-By: Claude Sonnet 4.5 --- packages/omnium-gatherum/package.json | 1 + packages/omnium-gatherum/src/background.ts | 4 +- .../src/controllers/base-controller.test.ts | 289 ++++++++++++++++++ .../src/controllers/base-controller.ts | 131 ++++++++ .../caplet/caplet-controller.test.ts | 42 +-- .../controllers/caplet/caplet-controller.ts | 285 ++++++++++------- .../src/controllers/caplet/index.ts | 4 +- .../omnium-gatherum/src/controllers/facet.ts | 2 +- .../omnium-gatherum/src/controllers/index.ts | 9 +- .../omnium-gatherum/src/controllers/types.ts | 9 +- packages/omnium-gatherum/src/manifest.json | 2 +- .../omnium-gatherum/src/types/semver.d.ts | 7 + yarn.lock | 9 +- 13 files changed, 648 insertions(+), 146 deletions(-) create mode 100644 packages/omnium-gatherum/src/controllers/base-controller.test.ts create mode 100644 packages/omnium-gatherum/src/controllers/base-controller.ts create mode 100644 packages/omnium-gatherum/src/types/semver.d.ts diff --git a/packages/omnium-gatherum/package.json b/packages/omnium-gatherum/package.json index ee8f0e8b7..62f19937a 100644 --- a/packages/omnium-gatherum/package.json +++ b/packages/omnium-gatherum/package.json @@ -74,6 +74,7 @@ "@types/chrome": "^0.0.313", "@types/react": "^17.0.11", "@types/react-dom": "^17.0.11", + "@types/semver": "^7.7.1", "@types/webextension-polyfill": "^0", "@typescript-eslint/eslint-plugin": "^8.29.0", "@typescript-eslint/parser": "^8.29.0", diff --git a/packages/omnium-gatherum/src/background.ts b/packages/omnium-gatherum/src/background.ts index 2c5e62fde..e7e1bb0de 100644 --- a/packages/omnium-gatherum/src/background.ts +++ b/packages/omnium-gatherum/src/background.ts @@ -16,9 +16,9 @@ import type { ClusterConfig } from '@metamask/ocap-kernel'; import { ChromeRuntimeDuplexStream } from '@metamask/streams/browser'; import { + CapletController, makeChromeStorageAdapter, makeControllerStorage, - makeCapletController, } from './controllers/index.ts'; import type { CapletControllerState, @@ -140,7 +140,7 @@ async function main(): Promise { }); // Create CapletController with attenuated kernel access - const capletController = makeCapletController( + const capletController = CapletController.make( { logger: logger.subLogger({ tags: ['caplet'] }) }, { storage: capletStorage, diff --git a/packages/omnium-gatherum/src/controllers/base-controller.test.ts b/packages/omnium-gatherum/src/controllers/base-controller.test.ts new file mode 100644 index 000000000..d39abb533 --- /dev/null +++ b/packages/omnium-gatherum/src/controllers/base-controller.test.ts @@ -0,0 +1,289 @@ +import { makeDefaultExo } from '@metamask/kernel-utils/exo'; +import type { Logger } from '@metamask/logger'; +import { describe, it, expect, vi, beforeEach } from 'vitest'; + +import { Controller } from './base-controller.ts'; +import type { ControllerConfig } from './base-controller.ts'; +import type { ControllerStorage } from './storage/controller-storage.ts'; + +/** + * Test state for the concrete test controller. + */ +type TestState = { + items: Record; + count: number; +}; + +/** + * Test methods for the concrete test controller. + */ +type TestMethods = { + addItem: (id: string, name: string, value: number) => Promise; + removeItem: (id: string) => Promise; + getItem: (id: string) => Promise<{ name: string; value: number } | undefined>; + getCount: () => Promise; +}; + +/** + * Concrete controller for testing the abstract Controller base class. + */ +class TestController extends Controller< + 'TestController', + TestState, + TestMethods +> { + // eslint-disable-next-line no-restricted-syntax -- TypeScript doesn't support # for constructors + private constructor(storage: ControllerStorage, logger: Logger) { + super('TestController', storage, logger); + harden(this); + } + + static create( + config: ControllerConfig, + storage: ControllerStorage, + ): TestMethods { + const controller = new TestController(storage, config.logger); + return controller.makeFacet(); + } + + makeFacet(): TestMethods { + return makeDefaultExo('TestController', { + addItem: async ( + id: string, + name: string, + value: number, + ): Promise => { + this.logger.info(`Adding item: ${id}`); + await this.update((draft) => { + draft.items[id] = { name, value }; + draft.count += 1; + }); + }, + removeItem: async (id: string): Promise => { + this.logger.info(`Removing item: ${id}`); + await this.update((draft) => { + delete draft.items[id]; + draft.count -= 1; + }); + }, + getItem: async ( + id: string, + ): Promise<{ name: string; value: number } | undefined> => { + return this.state.items[id]; + }, + getCount: async (): Promise => { + return this.state.count; + }, + }); + } +} +harden(TestController); + +/** + * Create a mock ControllerStorage for testing. + * + * @param initialState - The initial state for the mock storage. + * @returns A mock ControllerStorage instance with update tracking. + */ +function createMockStorage( + initialState: TestState, +): ControllerStorage & { updateCalls: (() => void)[] } { + let currentState = { ...initialState }; + const updateCalls: (() => void)[] = []; + + return { + get state(): Readonly { + return harden({ ...currentState }); + }, + + async update(producer: (draft: TestState) => void): Promise { + // Create a mutable draft + const draft = JSON.parse(JSON.stringify(currentState)) as TestState; + producer(draft); + currentState = draft; + updateCalls.push(() => producer(draft)); + }, + + async reload(): Promise { + // No-op for tests + }, + + updateCalls, + }; +} + +const emptyState: TestState = { + items: {}, + count: 0, +}; + +describe('Controller', () => { + const mockLogger = { + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + debug: vi.fn(), + subLogger: vi.fn().mockReturnThis(), + }; + + const config: ControllerConfig = { + logger: mockLogger as unknown as ControllerConfig['logger'], + }; + + beforeEach(() => { + vi.clearAllMocks(); + }); + + describe('state access', () => { + it('provides read-only access to state', async () => { + const initialState: TestState = { + items: { foo: { name: 'Foo', value: 42 } }, + count: 1, + }; + const mockStorage = createMockStorage(initialState); + const controller = TestController.create(config, mockStorage); + + const item = await controller.getItem('foo'); + + expect(item).toStrictEqual({ name: 'Foo', value: 42 }); + }); + + it('returns undefined for non-existent items', async () => { + const mockStorage = createMockStorage(emptyState); + const controller = TestController.create(config, mockStorage); + + const item = await controller.getItem('nonexistent'); + + expect(item).toBeUndefined(); + }); + + it('reflects initial state count', async () => { + const initialState: TestState = { + items: { + a: { name: 'A', value: 1 }, + b: { name: 'B', value: 2 }, + }, + count: 2, + }; + const mockStorage = createMockStorage(initialState); + const controller = TestController.create(config, mockStorage); + + const count = await controller.getCount(); + + expect(count).toBe(2); + }); + }); + + describe('state updates', () => { + it('updates state through update method', async () => { + const mockStorage = createMockStorage(emptyState); + const controller = TestController.create(config, mockStorage); + + await controller.addItem('test', 'Test Item', 100); + + const item = await controller.getItem('test'); + expect(item).toStrictEqual({ name: 'Test Item', value: 100 }); + }); + + it('increments count when adding items', async () => { + const mockStorage = createMockStorage(emptyState); + const controller = TestController.create(config, mockStorage); + + await controller.addItem('a', 'Item A', 1); + await controller.addItem('b', 'Item B', 2); + + const count = await controller.getCount(); + expect(count).toBe(2); + }); + + it('decrements count when removing items', async () => { + const initialState: TestState = { + items: { + a: { name: 'A', value: 1 }, + b: { name: 'B', value: 2 }, + }, + count: 2, + }; + const mockStorage = createMockStorage(initialState); + const controller = TestController.create(config, mockStorage); + + await controller.removeItem('a'); + + const count = await controller.getCount(); + expect(count).toBe(1); + }); + + it('removes item from state', async () => { + const initialState: TestState = { + items: { foo: { name: 'Foo', value: 42 } }, + count: 1, + }; + const mockStorage = createMockStorage(initialState); + const controller = TestController.create(config, mockStorage); + + await controller.removeItem('foo'); + + const item = await controller.getItem('foo'); + expect(item).toBeUndefined(); + }); + + it('calls storage.update for each state modification', async () => { + const mockStorage = createMockStorage(emptyState); + const controller = TestController.create(config, mockStorage); + + await controller.addItem('a', 'A', 1); + await controller.addItem('b', 'B', 2); + await controller.removeItem('a'); + + expect(mockStorage.updateCalls).toHaveLength(3); + }); + }); + + describe('logging', () => { + it('logs through provided logger', async () => { + const mockStorage = createMockStorage(emptyState); + const controller = TestController.create(config, mockStorage); + + await controller.addItem('test', 'Test', 1); + + expect(mockLogger.info).toHaveBeenCalledWith('Adding item: test'); + }); + + it('logs remove operations', async () => { + const initialState: TestState = { + items: { foo: { name: 'Foo', value: 42 } }, + count: 1, + }; + const mockStorage = createMockStorage(initialState); + const controller = TestController.create(config, mockStorage); + + await controller.removeItem('foo'); + + expect(mockLogger.info).toHaveBeenCalledWith('Removing item: foo'); + }); + }); + + describe('getMethods', () => { + it('returns hardened exo with all methods', async () => { + const mockStorage = createMockStorage(emptyState); + const methods = TestController.create(config, mockStorage); + + expect(typeof methods.addItem).toBe('function'); + expect(typeof methods.removeItem).toBe('function'); + expect(typeof methods.getItem).toBe('function'); + expect(typeof methods.getCount).toBe('function'); + }); + + it('methods work correctly through exo', async () => { + const mockStorage = createMockStorage(emptyState); + const methods = TestController.create(config, mockStorage); + + await methods.addItem('x', 'X', 10); + const item = await methods.getItem('x'); + const count = await methods.getCount(); + + expect(item).toStrictEqual({ name: 'X', value: 10 }); + expect(count).toBe(1); + }); + }); +}); diff --git a/packages/omnium-gatherum/src/controllers/base-controller.ts b/packages/omnium-gatherum/src/controllers/base-controller.ts new file mode 100644 index 000000000..c69a52b04 --- /dev/null +++ b/packages/omnium-gatherum/src/controllers/base-controller.ts @@ -0,0 +1,131 @@ +import type { Logger } from '@metamask/logger'; +import type { Json } from '@metamask/utils'; + +import type { ControllerStorage } from './storage/controller-storage.ts'; + +/** + * Base type for controller methods. + * Controllers expose their public API through a methods object. + */ +export type ControllerMethods = Record unknown>; + +/** + * Configuration passed to all controllers during initialization. + */ +export type ControllerConfig = { + logger: Logger; +}; + +/** + * Abstract base class for controllers. + * + * Provides state management via ControllerStorage with: + * - Synchronous state access via `this.state` + * - Async state updates via `this.update()` + * - Automatic persistence handled by storage layer + * + * Subclasses must: + * - Call `super()` in constructor with name, storage, and logger + * - Call `harden(this)` at the end of their constructor + * - Implement `getMethods()` to return a hardened exo with public API + * + * @template ControllerName - Literal string type for the controller name + * @template State - The state object shape (must be JSON-serializable) + * @template Methods - The public method interface + * + * @example + * ```typescript + * class MyController extends Controller<'MyController', MyState, MyMethods> { + * private constructor(storage: ControllerStorage, logger: Logger) { + * super('MyController', storage, logger); + * harden(this); + * } + * + * static create(config: ControllerConfig, deps: MyDeps): MyMethods { + * const controller = new MyController(deps.storage, config.logger); + * return controller.getMethods(); + * } + * + * getMethods(): MyMethods { + * return makeDefaultExo('MyController', { ... }); + * } + * } + * ``` + */ +export abstract class Controller< + ControllerName extends string, + State extends Record, + Methods extends ControllerMethods, +> { + readonly #name: ControllerName; + + readonly #storage: ControllerStorage; + + readonly #logger: Logger; + + /** + * Protected constructor - subclasses must call this via super(). + * + * @param name - Controller name for debugging/logging. + * @param storage - ControllerStorage instance for state management. + * @param logger - Logger instance. + */ + protected constructor( + name: ControllerName, + storage: ControllerStorage, + logger: Logger, + ) { + this.#name = name; + this.#storage = storage; + this.#logger = logger; + // Note: Subclass must call harden(this) after its own initialization + } + + /** + * Controller name for debugging/logging. + * + * @returns The controller name. + */ + protected get name(): ControllerName { + return this.#name; + } + + /** + * Current state (readonly). + * Provides synchronous access to in-memory state. + * + * @returns The current readonly state. + */ + protected get state(): Readonly { + return this.#storage.state; + } + + /** + * Logger instance for this controller. + * + * @returns The logger instance. + */ + protected get logger(): Logger { + return this.#logger; + } + + /** + * Update state using an immer producer function. + * Persistence is handled automatically by the storage layer. + * + * @param producer - Function that mutates a draft of the state. + * @returns Promise that resolves when changes are persisted. + */ + protected async update(producer: (draft: State) => void): Promise { + await this.#storage.update(producer); + } + + /** + * Returns the hardened exo with public methods. + * Subclasses implement this to define their public interface. + * + * @returns A hardened exo object with the controller's public methods. + */ + abstract makeFacet(): Methods; +} +harden(Controller); diff --git a/packages/omnium-gatherum/src/controllers/caplet/caplet-controller.test.ts b/packages/omnium-gatherum/src/controllers/caplet/caplet-controller.test.ts index 23b99df3a..7d61afd79 100644 --- a/packages/omnium-gatherum/src/controllers/caplet/caplet-controller.test.ts +++ b/packages/omnium-gatherum/src/controllers/caplet/caplet-controller.test.ts @@ -1,6 +1,6 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; -import { makeCapletController } from './caplet-controller.ts'; +import { CapletController } from './caplet-controller.ts'; import type { CapletControllerState } from './caplet-controller.ts'; import type { CapletManifest } from './types.ts'; import type { ControllerStorage } from '../storage/controller-storage.ts'; @@ -48,7 +48,7 @@ const emptyState: CapletControllerState = { caplets: {}, }; -describe('makeCapletController', () => { +describe('CapletController.make', () => { const mockLogger = { info: vi.fn(), warn: vi.fn(), @@ -83,7 +83,7 @@ describe('makeCapletController', () => { describe('install', () => { it('installs a caplet successfully', async () => { const mockStorage = createMockStorage(emptyState); - const controller = makeCapletController(config, { + const controller = CapletController.make(config, { storage: mockStorage, launchSubcluster: mockLaunchSubcluster, terminateSubcluster: mockTerminateSubcluster, @@ -99,7 +99,7 @@ describe('makeCapletController', () => { it('validates the manifest', async () => { const mockStorage = createMockStorage(emptyState); - const controller = makeCapletController(config, { + const controller = CapletController.make(config, { storage: mockStorage, launchSubcluster: mockLaunchSubcluster, terminateSubcluster: mockTerminateSubcluster, @@ -123,7 +123,7 @@ describe('makeCapletController', () => { }, }; const mockStorage = createMockStorage(stateWithCaplet); - const controller = makeCapletController(config, { + const controller = CapletController.make(config, { storage: mockStorage, launchSubcluster: mockLaunchSubcluster, terminateSubcluster: mockTerminateSubcluster, @@ -136,7 +136,7 @@ describe('makeCapletController', () => { it('launches subcluster with correct config', async () => { const mockStorage = createMockStorage(emptyState); - const controller = makeCapletController(config, { + const controller = CapletController.make(config, { storage: mockStorage, launchSubcluster: mockLaunchSubcluster, terminateSubcluster: mockTerminateSubcluster, @@ -159,7 +159,7 @@ describe('makeCapletController', () => { vi.setSystemTime(new Date('2024-01-15T12:00:00Z')); const mockStorage = createMockStorage(emptyState); - const controller = makeCapletController(config, { + const controller = CapletController.make(config, { storage: mockStorage, launchSubcluster: mockLaunchSubcluster, terminateSubcluster: mockTerminateSubcluster, @@ -187,7 +187,7 @@ describe('makeCapletController', () => { }, }; const mockStorage = createMockStorage(stateWithOtherCaplet); - const controller = makeCapletController(config, { + const controller = CapletController.make(config, { storage: mockStorage, launchSubcluster: mockLaunchSubcluster, terminateSubcluster: mockTerminateSubcluster, @@ -203,7 +203,7 @@ describe('makeCapletController', () => { it('logs installation progress', async () => { const mockStorage = createMockStorage(emptyState); - const controller = makeCapletController(config, { + const controller = CapletController.make(config, { storage: mockStorage, launchSubcluster: mockLaunchSubcluster, terminateSubcluster: mockTerminateSubcluster, @@ -232,7 +232,7 @@ describe('makeCapletController', () => { }, }; const mockStorage = createMockStorage(stateWithCaplet); - const controller = makeCapletController(config, { + const controller = CapletController.make(config, { storage: mockStorage, launchSubcluster: mockLaunchSubcluster, terminateSubcluster: mockTerminateSubcluster, @@ -245,7 +245,7 @@ describe('makeCapletController', () => { it('throws if caplet not found', async () => { const mockStorage = createMockStorage(emptyState); - const controller = makeCapletController(config, { + const controller = CapletController.make(config, { storage: mockStorage, launchSubcluster: mockLaunchSubcluster, terminateSubcluster: mockTerminateSubcluster, @@ -267,7 +267,7 @@ describe('makeCapletController', () => { }, }; const mockStorage = createMockStorage(stateWithCaplet); - const controller = makeCapletController(config, { + const controller = CapletController.make(config, { storage: mockStorage, launchSubcluster: mockLaunchSubcluster, terminateSubcluster: mockTerminateSubcluster, @@ -294,7 +294,7 @@ describe('makeCapletController', () => { }, }; const mockStorage = createMockStorage(stateWithCaplets); - const controller = makeCapletController(config, { + const controller = CapletController.make(config, { storage: mockStorage, launchSubcluster: mockLaunchSubcluster, terminateSubcluster: mockTerminateSubcluster, @@ -318,7 +318,7 @@ describe('makeCapletController', () => { }, }; const mockStorage = createMockStorage(stateWithCaplet); - const controller = makeCapletController(config, { + const controller = CapletController.make(config, { storage: mockStorage, launchSubcluster: mockLaunchSubcluster, terminateSubcluster: mockTerminateSubcluster, @@ -338,7 +338,7 @@ describe('makeCapletController', () => { describe('list', () => { it('returns empty array when no caplets installed', async () => { const mockStorage = createMockStorage(emptyState); - const controller = makeCapletController(config, { + const controller = CapletController.make(config, { storage: mockStorage, launchSubcluster: mockLaunchSubcluster, terminateSubcluster: mockTerminateSubcluster, @@ -370,7 +370,7 @@ describe('makeCapletController', () => { }, }; const mockStorage = createMockStorage(stateWithCaplets); - const controller = makeCapletController(config, { + const controller = CapletController.make(config, { storage: mockStorage, launchSubcluster: mockLaunchSubcluster, terminateSubcluster: mockTerminateSubcluster, @@ -404,7 +404,7 @@ describe('makeCapletController', () => { }, }; const mockStorage = createMockStorage(stateWithCaplet); - const controller = makeCapletController(config, { + const controller = CapletController.make(config, { storage: mockStorage, launchSubcluster: mockLaunchSubcluster, terminateSubcluster: mockTerminateSubcluster, @@ -421,7 +421,7 @@ describe('makeCapletController', () => { it('returns undefined if caplet not found', async () => { const mockStorage = createMockStorage(emptyState); - const controller = makeCapletController(config, { + const controller = CapletController.make(config, { storage: mockStorage, launchSubcluster: mockLaunchSubcluster, terminateSubcluster: mockTerminateSubcluster, @@ -445,7 +445,7 @@ describe('makeCapletController', () => { }, }; const mockStorage = createMockStorage(stateWithCaplet); - const controller = makeCapletController(config, { + const controller = CapletController.make(config, { storage: mockStorage, launchSubcluster: mockLaunchSubcluster, terminateSubcluster: mockTerminateSubcluster, @@ -468,7 +468,7 @@ describe('makeCapletController', () => { }, }; const mockStorage = createMockStorage(stateWithCaplet); - const controller = makeCapletController(config, { + const controller = CapletController.make(config, { storage: mockStorage, launchSubcluster: mockLaunchSubcluster, terminateSubcluster: mockTerminateSubcluster, @@ -501,7 +501,7 @@ describe('makeCapletController', () => { }, }; const mockStorage = createMockStorage(stateWithCaplets); - const controller = makeCapletController(config, { + const controller = CapletController.make(config, { storage: mockStorage, launchSubcluster: mockLaunchSubcluster, terminateSubcluster: mockTerminateSubcluster, diff --git a/packages/omnium-gatherum/src/controllers/caplet/caplet-controller.ts b/packages/omnium-gatherum/src/controllers/caplet/caplet-controller.ts index 8c7b8a700..584694631 100644 --- a/packages/omnium-gatherum/src/controllers/caplet/caplet-controller.ts +++ b/packages/omnium-gatherum/src/controllers/caplet/caplet-controller.ts @@ -1,4 +1,5 @@ import { makeDefaultExo } from '@metamask/kernel-utils/exo'; +import type { Logger } from '@metamask/logger'; import type { ClusterConfig } from '@metamask/ocap-kernel'; import type { @@ -9,8 +10,9 @@ import type { LaunchResult, } from './types.ts'; import { isCapletManifest } from './types.ts'; +import { Controller } from '../base-controller.ts'; +import type { ControllerConfig } from '../base-controller.ts'; import type { ControllerStorage } from '../storage/controller-storage.ts'; -import type { ControllerConfig } from '../types.ts'; /** * Caplet controller persistent state. @@ -25,7 +27,7 @@ export type CapletControllerState = { /** * Methods exposed by the CapletController. */ -export type CapletControllerMethods = { +export type CapletControllerFacet = { /** * Install a caplet. * @@ -83,122 +85,197 @@ export type CapletControllerDeps = { }; /** - * Create the CapletController. + * Controller for managing caplet lifecycle. * - * The CapletController manages the lifecycle of installed caplets: + * The CapletController manages: * - Installing caplets (validating manifest, launching subcluster, storing metadata) * - Uninstalling caplets (terminating subcluster, removing metadata) * - Querying installed caplets - * - * @param config - Controller configuration. - * @param deps - Controller dependencies (attenuated for POLA). - * @returns A hardened CapletController exo. */ -export function makeCapletController( - config: ControllerConfig, - deps: CapletControllerDeps, -): CapletControllerMethods { - const { logger } = config; - const { storage, launchSubcluster, terminateSubcluster } = deps; +export class CapletController extends Controller< + 'CapletController', + CapletControllerState, + CapletControllerFacet +> { + readonly #launchSubcluster: (config: ClusterConfig) => Promise; + + readonly #terminateSubcluster: (subclusterId: string) => Promise; /** - * Get an installed caplet by ID (synchronous - reads from in-memory state). + * Private constructor - use static create() method. * - * @param capletId - The caplet ID to retrieve. - * @returns The installed caplet or undefined if not found. + * @param storage - ControllerStorage for caplet state. + * @param logger - Logger instance. + * @param launchSubcluster - Function to launch a subcluster. + * @param terminateSubcluster - Function to terminate a subcluster. */ - const getCaplet = (capletId: CapletId): InstalledCaplet | undefined => { - return storage.state.caplets[capletId]; - }; + // eslint-disable-next-line no-restricted-syntax -- TypeScript doesn't support # for constructors + private constructor( + storage: ControllerStorage, + logger: Logger, + launchSubcluster: (config: ClusterConfig) => Promise, + terminateSubcluster: (subclusterId: string) => Promise, + ) { + super('CapletController', storage, logger); + this.#launchSubcluster = launchSubcluster; + this.#terminateSubcluster = terminateSubcluster; + harden(this); + } /** - * Get all installed caplets (synchronous - reads from in-memory state). + * Create a CapletController and return its public methods. * - * @returns Array of all installed caplets. + * @param config - Controller configuration. + * @param deps - Controller dependencies (attenuated for POLA). + * @returns A hardened CapletController exo. + */ + static make( + config: ControllerConfig, + deps: CapletControllerDeps, + ): CapletControllerFacet { + const controller = new CapletController( + deps.storage, + config.logger, + deps.launchSubcluster, + deps.terminateSubcluster, + ); + return controller.makeFacet(); + } + + /** + * Returns the hardened exo with public methods. + * + * @returns A hardened exo object with the controller's public methods. + */ + makeFacet(): CapletControllerFacet { + return makeDefaultExo('CapletController', { + install: async ( + manifest: CapletManifest, + _bundle?: unknown, + ): Promise => { + return this.#install(manifest, _bundle); + }, + uninstall: async (capletId: CapletId): Promise => { + return this.#uninstall(capletId); + }, + list: async (): Promise => { + return this.#list(); + }, + get: async (capletId: CapletId): Promise => { + return this.#get(capletId); + }, + getByService: async ( + serviceName: string, + ): Promise => { + return this.#getByService(serviceName); + }, + }); + } + + /** + * Install a caplet. + * + * @param manifest - The caplet manifest. + * @param _bundle - The caplet bundle (currently unused). + * @returns The installation result. */ - const listCaplets = (): InstalledCaplet[] => { - return Object.values(storage.state.caplets); - }; - - return makeDefaultExo('CapletController', { - async install( - manifest: CapletManifest, - _bundle?: unknown, - ): Promise { - const { id } = manifest; - logger.info(`Installing caplet: ${id}`); - - // Validate manifest - if (!isCapletManifest(manifest)) { - throw new Error(`Invalid caplet manifest for ${id}`); - } - - // Check if already installed - if (storage.state.caplets[id] !== undefined) { - throw new Error(`Caplet ${id} is already installed`); - } - - // Create cluster config for this caplet - const clusterConfig: ClusterConfig = { - bootstrap: id, - vats: { - [id]: { - bundleSpec: manifest.bundleSpec, - }, + async #install( + manifest: CapletManifest, + _bundle?: unknown, + ): Promise { + const { id } = manifest; + this.logger.info(`Installing caplet: ${id}`); + + // Validate manifest + if (!isCapletManifest(manifest)) { + throw new Error(`Invalid caplet manifest for ${id}`); + } + + // Check if already installed + if (this.state.caplets[id] !== undefined) { + throw new Error(`Caplet ${id} is already installed`); + } + + // Create cluster config for this caplet + const clusterConfig: ClusterConfig = { + bootstrap: id, + vats: { + [id]: { + bundleSpec: manifest.bundleSpec, }, + }, + }; + + // Launch subcluster + const { subclusterId } = await this.#launchSubcluster(clusterConfig); + + // Store caplet data + await this.update((draft) => { + draft.caplets[id] = { + manifest, + subclusterId, + installedAt: Date.now(), }; + }); - // Launch subcluster - const { subclusterId } = await launchSubcluster(clusterConfig); - - // Store caplet data - await storage.update((draft) => { - draft.caplets[id] = { - manifest, - subclusterId, - installedAt: Date.now(), - }; - }); - - logger.info(`Caplet ${id} installed with subcluster ${subclusterId}`); - return { capletId: id, subclusterId }; - }, - - async uninstall(capletId: CapletId): Promise { - logger.info(`Uninstalling caplet: ${capletId}`); - - const caplet = storage.state.caplets[capletId]; - if (caplet === undefined) { - throw new Error(`Caplet ${capletId} not found`); - } - - // Terminate the subcluster - await terminateSubcluster(caplet.subclusterId); - - // Remove from storage - await storage.update((draft) => { - delete draft.caplets[capletId]; - }); - - logger.info(`Caplet ${capletId} uninstalled`); - }, - - async list(): Promise { - return listCaplets(); - }, - - async get(capletId: CapletId): Promise { - return getCaplet(capletId); - }, - - async getByService( - serviceName: string, - ): Promise { - const caplets = listCaplets(); - return caplets.find((caplet: InstalledCaplet) => - caplet.manifest.providedServices.includes(serviceName), - ); - }, - }); + this.logger.info(`Caplet ${id} installed with subcluster ${subclusterId}`); + return { capletId: id, subclusterId }; + } + + /** + * Uninstall a caplet. + * + * @param capletId - The ID of the caplet to uninstall. + */ + async #uninstall(capletId: CapletId): Promise { + this.logger.info(`Uninstalling caplet: ${capletId}`); + + const caplet = this.state.caplets[capletId]; + if (caplet === undefined) { + throw new Error(`Caplet ${capletId} not found`); + } + + // Terminate the subcluster + await this.#terminateSubcluster(caplet.subclusterId); + + // Remove from storage + await this.update((draft) => { + delete draft.caplets[capletId]; + }); + + this.logger.info(`Caplet ${capletId} uninstalled`); + } + + /** + * Get all installed caplets. + * + * @returns Array of all installed caplets. + */ + #list(): InstalledCaplet[] { + return Object.values(this.state.caplets); + } + + /** + * Get an installed caplet by ID. + * + * @param capletId - The caplet ID to retrieve. + * @returns The installed caplet or undefined if not found. + */ + #get(capletId: CapletId): InstalledCaplet | undefined { + return this.state.caplets[capletId]; + } + + /** + * Find a caplet that provides a specific service. + * + * @param serviceName - The service name to search for. + * @returns The installed caplet or undefined if not found. + */ + #getByService(serviceName: string): InstalledCaplet | undefined { + const caplets = this.#list(); + return caplets.find((caplet: InstalledCaplet) => + caplet.manifest.providedServices.includes(serviceName), + ); + } } -harden(makeCapletController); +harden(CapletController); diff --git a/packages/omnium-gatherum/src/controllers/caplet/index.ts b/packages/omnium-gatherum/src/controllers/caplet/index.ts index e0cb3f5cf..af216b869 100644 --- a/packages/omnium-gatherum/src/controllers/caplet/index.ts +++ b/packages/omnium-gatherum/src/controllers/caplet/index.ts @@ -16,8 +16,8 @@ export { CapletManifestStruct, } from './types.ts'; export type { - CapletControllerMethods, + CapletControllerFacet, CapletControllerDeps, CapletControllerState, } from './caplet-controller.ts'; -export { makeCapletController } from './caplet-controller.ts'; +export { CapletController } from './caplet-controller.ts'; diff --git a/packages/omnium-gatherum/src/controllers/facet.ts b/packages/omnium-gatherum/src/controllers/facet.ts index 2ec6dc269..1825ceebd 100644 --- a/packages/omnium-gatherum/src/controllers/facet.ts +++ b/packages/omnium-gatherum/src/controllers/facet.ts @@ -35,7 +35,7 @@ type FacetMethods> = Methods & { * * // CapletController only needs get/set, not clear/getAll * const storageFacet = makeFacet('CapletStorage', storageController, ['get', 'set']); - * const capletController = makeCapletController({ storage: storageFacet }); + * const capletController = CapletController.make({ storage: storageFacet }); * ``` */ export function makeFacet< diff --git a/packages/omnium-gatherum/src/controllers/index.ts b/packages/omnium-gatherum/src/controllers/index.ts index 22abb8f00..cc6326308 100644 --- a/packages/omnium-gatherum/src/controllers/index.ts +++ b/packages/omnium-gatherum/src/controllers/index.ts @@ -1,5 +1,6 @@ -// Base types -export type { ControllerConfig, FacetOf } from './types.ts'; +// Base controller +export { Controller } from './base-controller.ts'; +export type { ControllerConfig, ControllerMethods, FacetOf } from './types.ts'; export { makeFacet } from './facet.ts'; // Storage @@ -24,7 +25,7 @@ export type { InstallResult, LaunchResult, CapletControllerState, - CapletControllerMethods, + CapletControllerFacet, CapletControllerDeps, } from './caplet/index.ts'; export { @@ -35,5 +36,5 @@ export { CapletIdStruct, SemVerStruct, CapletManifestStruct, - makeCapletController, + CapletController, } from './caplet/index.ts'; diff --git a/packages/omnium-gatherum/src/controllers/types.ts b/packages/omnium-gatherum/src/controllers/types.ts index 2c4cfc890..84f2287e4 100644 --- a/packages/omnium-gatherum/src/controllers/types.ts +++ b/packages/omnium-gatherum/src/controllers/types.ts @@ -1,12 +1,7 @@ import type { Methods } from '@endo/exo'; -import type { Logger } from '@metamask/logger'; -/** - * Configuration passed to all controllers during initialization. - */ -export type ControllerConfig = { - logger: Logger; -}; +// Re-export from base-controller for backward compatibility +export type { ControllerConfig, ControllerMethods } from './base-controller.ts'; /** * Type helper for defining facet interfaces. diff --git a/packages/omnium-gatherum/src/manifest.json b/packages/omnium-gatherum/src/manifest.json index 8f815cecd..653d0b8bd 100644 --- a/packages/omnium-gatherum/src/manifest.json +++ b/packages/omnium-gatherum/src/manifest.json @@ -10,7 +10,7 @@ "action": { "default_popup": "popup.html" }, - "permissions": ["offscreen", "unlimitedStorage"], + "permissions": ["offscreen", "storage", "unlimitedStorage"], "sandbox": { "pages": ["iframe.html"] }, diff --git a/packages/omnium-gatherum/src/types/semver.d.ts b/packages/omnium-gatherum/src/types/semver.d.ts new file mode 100644 index 000000000..9a6ab706d --- /dev/null +++ b/packages/omnium-gatherum/src/types/semver.d.ts @@ -0,0 +1,7 @@ +declare module 'semver/functions/valid' { + function valid( + version: string | null | undefined, + optionsOrLoose?: boolean | { loose?: boolean; includePrerelease?: boolean }, + ): string | null; + export default valid; +} diff --git a/yarn.lock b/yarn.lock index c8e29036d..a41a32194 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3941,6 +3941,7 @@ __metadata: "@types/chrome": "npm:^0.0.313" "@types/react": "npm:^17.0.11" "@types/react-dom": "npm:^17.0.11" + "@types/semver": "npm:^7.7.1" "@types/webextension-polyfill": "npm:^0" "@typescript-eslint/eslint-plugin": "npm:^8.29.0" "@typescript-eslint/parser": "npm:^8.29.0" @@ -5428,10 +5429,10 @@ __metadata: languageName: node linkType: hard -"@types/semver@npm:^7.3.6": - version: 7.7.0 - resolution: "@types/semver@npm:7.7.0" - checksum: 10/ee4514c6c852b1c38f951239db02f9edeea39f5310fad9396a00b51efa2a2d96b3dfca1ae84c88181ea5b7157c57d32d7ef94edacee36fbf975546396b85ba5b +"@types/semver@npm:^7.3.6, @types/semver@npm:^7.7.1": + version: 7.7.1 + resolution: "@types/semver@npm:7.7.1" + checksum: 10/8f09e7e6ca3ded67d78ba7a8f7535c8d9cf8ced83c52e7f3ac3c281fe8c689c3fe475d199d94390dc04fc681d51f2358b430bb7b2e21c62de24f2bee2c719068 languageName: node linkType: hard From 14d507a7ad2d655b894523c7f61f4095c2196727 Mon Sep 17 00:00:00 2001 From: Erik Marks <25517051+rekmarks@users.noreply.github.com> Date: Fri, 9 Jan 2026 15:38:55 -0800 Subject: [PATCH 4/7] refactor(omnium): Refactor ControllerStorage with debounced persistence - Convert ControllerStorage from factory to class with static make() method - Implement synchronous update() with debounced fire-and-forget persistence - Fix critical debounce bug: accumulate modified keys across debounce window - Implement bounded latency (timer not reset, max delay = one debounce interval) - Add immediate writes when idle > debounceMs for better responsiveness - Add clear() and clearState() methods to reset storage to defaults - Remove old namespaced-storage implementation - Refactor all tests to use actual ControllerStorage with mock adapters - Add shared makeMockStorageAdapter() utility in test/utils.ts - Update controllers to create their own storage from adapters Co-Authored-By: Claude Sonnet 4.5 --- packages/omnium-gatherum/src/background.ts | 22 +- .../src/controllers/base-controller.test.ts | 234 ++++++----- .../src/controllers/base-controller.ts | 21 +- .../caplet/caplet-controller.test.ts | 367 ++++++++---------- .../controllers/caplet/caplet-controller.ts | 27 +- .../omnium-gatherum/src/controllers/index.ts | 4 +- .../storage/controller-storage.test.ts | 301 +++++++++++--- .../controllers/storage/controller-storage.ts | 341 ++++++++++------ .../src/controllers/storage/index.ts | 8 +- .../storage/namespaced-storage.test.ts | 156 -------- .../controllers/storage/namespaced-storage.ts | 52 --- .../omnium-gatherum/test/e2e/smoke.test.ts | 2 +- .../test/{helpers.ts => e2e/utils.ts} | 2 +- packages/omnium-gatherum/test/utils.ts | 31 ++ 14 files changed, 816 insertions(+), 752 deletions(-) delete mode 100644 packages/omnium-gatherum/src/controllers/storage/namespaced-storage.test.ts delete mode 100644 packages/omnium-gatherum/src/controllers/storage/namespaced-storage.ts rename packages/omnium-gatherum/test/{helpers.ts => e2e/utils.ts} (96%) create mode 100644 packages/omnium-gatherum/test/utils.ts diff --git a/packages/omnium-gatherum/src/background.ts b/packages/omnium-gatherum/src/background.ts index e7e1bb0de..091e74dde 100644 --- a/packages/omnium-gatherum/src/background.ts +++ b/packages/omnium-gatherum/src/background.ts @@ -18,13 +18,8 @@ import { ChromeRuntimeDuplexStream } from '@metamask/streams/browser'; import { CapletController, makeChromeStorageAdapter, - makeControllerStorage, -} from './controllers/index.ts'; -import type { - CapletControllerState, - CapletManifest, - LaunchResult, } from './controllers/index.ts'; +import type { CapletManifest, LaunchResult } from './controllers/index.ts'; defineGlobals(); @@ -128,22 +123,15 @@ async function main(): Promise { return kernelP; }; - // Create storage adapter and state storage for caplets + // Create storage adapter const storageAdapter = makeChromeStorageAdapter(); - const defaultCapletState: CapletControllerState = { - caplets: {}, - }; - const capletStorage = await makeControllerStorage({ - namespace: 'caplet', - adapter: storageAdapter, - defaultState: defaultCapletState, - }); // Create CapletController with attenuated kernel access - const capletController = CapletController.make( + // Controller creates its own storage internally + const capletController = await CapletController.make( { logger: logger.subLogger({ tags: ['caplet'] }) }, { - storage: capletStorage, + adapter: storageAdapter, // Wrap launchSubcluster to return subclusterId launchSubcluster: async ( config: ClusterConfig, diff --git a/packages/omnium-gatherum/src/controllers/base-controller.test.ts b/packages/omnium-gatherum/src/controllers/base-controller.test.ts index d39abb533..1bfa82f9b 100644 --- a/packages/omnium-gatherum/src/controllers/base-controller.test.ts +++ b/packages/omnium-gatherum/src/controllers/base-controller.test.ts @@ -4,7 +4,9 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; import { Controller } from './base-controller.ts'; import type { ControllerConfig } from './base-controller.ts'; -import type { ControllerStorage } from './storage/controller-storage.ts'; +import { ControllerStorage } from './storage/controller-storage.ts'; +import type { StorageAdapter } from './storage/types.ts'; +import { makeMockStorageAdapter } from '../../test/utils.ts'; /** * Test state for the concrete test controller. @@ -22,6 +24,8 @@ type TestMethods = { removeItem: (id: string) => Promise; getItem: (id: string) => Promise<{ name: string; value: number } | undefined>; getCount: () => Promise; + clearState: () => void; + getState: () => Readonly; }; /** @@ -38,10 +42,21 @@ class TestController extends Controller< harden(this); } - static create( + static async make( config: ControllerConfig, - storage: ControllerStorage, - ): TestMethods { + adapter: StorageAdapter, + ): Promise { + const storage = await ControllerStorage.make({ + namespace: 'test', + adapter, + defaultState: { + items: {}, + count: 0, + }, + logger: config.logger, + debounceMs: 0, + }); + const controller = new TestController(storage, config.logger); return controller.makeFacet(); } @@ -54,14 +69,14 @@ class TestController extends Controller< value: number, ): Promise => { this.logger.info(`Adding item: ${id}`); - await this.update((draft) => { + this.update((draft) => { draft.items[id] = { name, value }; draft.count += 1; }); }, removeItem: async (id: string): Promise => { this.logger.info(`Removing item: ${id}`); - await this.update((draft) => { + this.update((draft) => { delete draft.items[id]; draft.count -= 1; }); @@ -74,49 +89,17 @@ class TestController extends Controller< getCount: async (): Promise => { return this.state.count; }, + clearState: (): void => { + this.clearState(); + }, + getState: (): Readonly => { + return this.state; + }, }); } } harden(TestController); -/** - * Create a mock ControllerStorage for testing. - * - * @param initialState - The initial state for the mock storage. - * @returns A mock ControllerStorage instance with update tracking. - */ -function createMockStorage( - initialState: TestState, -): ControllerStorage & { updateCalls: (() => void)[] } { - let currentState = { ...initialState }; - const updateCalls: (() => void)[] = []; - - return { - get state(): Readonly { - return harden({ ...currentState }); - }, - - async update(producer: (draft: TestState) => void): Promise { - // Create a mutable draft - const draft = JSON.parse(JSON.stringify(currentState)) as TestState; - producer(draft); - currentState = draft; - updateCalls.push(() => producer(draft)); - }, - - async reload(): Promise { - // No-op for tests - }, - - updateCalls, - }; -} - -const emptyState: TestState = { - items: {}, - count: 0, -}; - describe('Controller', () => { const mockLogger = { info: vi.fn(), @@ -136,12 +119,11 @@ describe('Controller', () => { describe('state access', () => { it('provides read-only access to state', async () => { - const initialState: TestState = { - items: { foo: { name: 'Foo', value: 42 } }, - count: 1, - }; - const mockStorage = createMockStorage(initialState); - const controller = TestController.create(config, mockStorage); + const mockAdapter = makeMockStorageAdapter(); + await mockAdapter.set('test.items', { foo: { name: 'Foo', value: 42 } }); + await mockAdapter.set('test.count', 1); + + const controller = await TestController.make(config, mockAdapter); const item = await controller.getItem('foo'); @@ -149,8 +131,8 @@ describe('Controller', () => { }); it('returns undefined for non-existent items', async () => { - const mockStorage = createMockStorage(emptyState); - const controller = TestController.create(config, mockStorage); + const mockAdapter = makeMockStorageAdapter(); + const controller = await TestController.make(config, mockAdapter); const item = await controller.getItem('nonexistent'); @@ -158,15 +140,14 @@ describe('Controller', () => { }); it('reflects initial state count', async () => { - const initialState: TestState = { - items: { - a: { name: 'A', value: 1 }, - b: { name: 'B', value: 2 }, - }, - count: 2, - }; - const mockStorage = createMockStorage(initialState); - const controller = TestController.create(config, mockStorage); + const mockAdapter = makeMockStorageAdapter(); + await mockAdapter.set('test.items', { + a: { name: 'A', value: 1 }, + b: { name: 'B', value: 2 }, + }); + await mockAdapter.set('test.count', 2); + + const controller = await TestController.make(config, mockAdapter); const count = await controller.getCount(); @@ -176,8 +157,8 @@ describe('Controller', () => { describe('state updates', () => { it('updates state through update method', async () => { - const mockStorage = createMockStorage(emptyState); - const controller = TestController.create(config, mockStorage); + const mockAdapter = makeMockStorageAdapter(); + const controller = await TestController.make(config, mockAdapter); await controller.addItem('test', 'Test Item', 100); @@ -186,8 +167,8 @@ describe('Controller', () => { }); it('increments count when adding items', async () => { - const mockStorage = createMockStorage(emptyState); - const controller = TestController.create(config, mockStorage); + const mockAdapter = makeMockStorageAdapter(); + const controller = await TestController.make(config, mockAdapter); await controller.addItem('a', 'Item A', 1); await controller.addItem('b', 'Item B', 2); @@ -197,15 +178,14 @@ describe('Controller', () => { }); it('decrements count when removing items', async () => { - const initialState: TestState = { - items: { - a: { name: 'A', value: 1 }, - b: { name: 'B', value: 2 }, - }, - count: 2, - }; - const mockStorage = createMockStorage(initialState); - const controller = TestController.create(config, mockStorage); + const mockAdapter = makeMockStorageAdapter(); + await mockAdapter.set('test.items', { + a: { name: 'A', value: 1 }, + b: { name: 'B', value: 2 }, + }); + await mockAdapter.set('test.count', 2); + + const controller = await TestController.make(config, mockAdapter); await controller.removeItem('a'); @@ -214,12 +194,11 @@ describe('Controller', () => { }); it('removes item from state', async () => { - const initialState: TestState = { - items: { foo: { name: 'Foo', value: 42 } }, - count: 1, - }; - const mockStorage = createMockStorage(initialState); - const controller = TestController.create(config, mockStorage); + const mockAdapter = makeMockStorageAdapter(); + await mockAdapter.set('test.items', { foo: { name: 'Foo', value: 42 } }); + await mockAdapter.set('test.count', 1); + + const controller = await TestController.make(config, mockAdapter); await controller.removeItem('foo'); @@ -227,22 +206,29 @@ describe('Controller', () => { expect(item).toBeUndefined(); }); - it('calls storage.update for each state modification', async () => { - const mockStorage = createMockStorage(emptyState); - const controller = TestController.create(config, mockStorage); + it('persists state modifications to storage', async () => { + const mockAdapter = makeMockStorageAdapter(); + const controller = await TestController.make(config, mockAdapter); await controller.addItem('a', 'A', 1); await controller.addItem('b', 'B', 2); await controller.removeItem('a'); - expect(mockStorage.updateCalls).toHaveLength(3); + // Wait for debounced persistence + await new Promise((resolve) => setTimeout(resolve, 10)); + + // Check that state was persisted + const items = await mockAdapter.get('test.items'); + const count = await mockAdapter.get('test.count'); + expect(items).toStrictEqual({ b: { name: 'B', value: 2 } }); + expect(count).toBe(1); }); }); describe('logging', () => { it('logs through provided logger', async () => { - const mockStorage = createMockStorage(emptyState); - const controller = TestController.create(config, mockStorage); + const mockAdapter = makeMockStorageAdapter(); + const controller = await TestController.make(config, mockAdapter); await controller.addItem('test', 'Test', 1); @@ -250,12 +236,11 @@ describe('Controller', () => { }); it('logs remove operations', async () => { - const initialState: TestState = { - items: { foo: { name: 'Foo', value: 42 } }, - count: 1, - }; - const mockStorage = createMockStorage(initialState); - const controller = TestController.create(config, mockStorage); + const mockAdapter = makeMockStorageAdapter(); + await mockAdapter.set('test.items', { foo: { name: 'Foo', value: 42 } }); + await mockAdapter.set('test.count', 1); + + const controller = await TestController.make(config, mockAdapter); await controller.removeItem('foo'); @@ -263,24 +248,63 @@ describe('Controller', () => { }); }); - describe('getMethods', () => { - it('returns hardened exo with all methods', async () => { - const mockStorage = createMockStorage(emptyState); - const methods = TestController.create(config, mockStorage); + describe('clearState', () => { + it('clears state through clearState method', async () => { + const mockAdapter = makeMockStorageAdapter(); + const controller = await TestController.make(config, mockAdapter); + await controller.addItem('a', 'A', 1); + + const stateBefore = controller.getState(); + expect(stateBefore.items).toStrictEqual({ a: { name: 'A', value: 1 } }); + expect(stateBefore.count).toBe(1); - expect(typeof methods.addItem).toBe('function'); - expect(typeof methods.removeItem).toBe('function'); - expect(typeof methods.getItem).toBe('function'); - expect(typeof methods.getCount).toBe('function'); + controller.clearState(); + + const stateAfter = controller.getState(); + expect(stateAfter.items).toStrictEqual({}); + expect(stateAfter.count).toBe(0); + }); + + it('persists cleared state', async () => { + const mockAdapter = makeMockStorageAdapter(); + const controller = await TestController.make(config, mockAdapter); + await controller.addItem('a', 'A', 1); + + // Wait for persistence + await new Promise((resolve) => setTimeout(resolve, 10)); + + controller.clearState(); + + // Wait for persistence + await new Promise((resolve) => setTimeout(resolve, 10)); + + const items = await mockAdapter.get('test.items'); + const count = await mockAdapter.get('test.count'); + expect(items).toStrictEqual({}); + expect(count).toBe(0); + }); + }); + + describe('makeFacet', () => { + it('returns hardened exo with all methods', async () => { + const mockAdapter = makeMockStorageAdapter(); + const facet = await TestController.make(config, mockAdapter); + + expect(typeof facet.addItem).toBe('function'); + expect(typeof facet.removeItem).toBe('function'); + expect(typeof facet.getItem).toBe('function'); + expect(typeof facet.getCount).toBe('function'); + expect(typeof facet.clearState).toBe('function'); + expect(typeof facet.getState).toBe('function'); }); it('methods work correctly through exo', async () => { - const mockStorage = createMockStorage(emptyState); - const methods = TestController.create(config, mockStorage); + const mockAdapter = makeMockStorageAdapter(); + const facet = await TestController.make(config, mockAdapter); - await methods.addItem('x', 'X', 10); - const item = await methods.getItem('x'); - const count = await methods.getCount(); + await facet.addItem('x', 'X', 10); + const item = await facet.getItem('x'); + const count = await facet.getCount(); expect(item).toStrictEqual({ name: 'X', value: 10 }); expect(count).toBe(1); diff --git a/packages/omnium-gatherum/src/controllers/base-controller.ts b/packages/omnium-gatherum/src/controllers/base-controller.ts index c69a52b04..5b049576f 100644 --- a/packages/omnium-gatherum/src/controllers/base-controller.ts +++ b/packages/omnium-gatherum/src/controllers/base-controller.ts @@ -27,7 +27,7 @@ export type ControllerConfig = { * Subclasses must: * - Call `super()` in constructor with name, storage, and logger * - Call `harden(this)` at the end of their constructor - * - Implement `getMethods()` to return a hardened exo with public API + * - Implement `makeFacet()` to return a hardened exo with public API * * @template ControllerName - Literal string type for the controller name * @template State - The state object shape (must be JSON-serializable) @@ -43,10 +43,10 @@ export type ControllerConfig = { * * static create(config: ControllerConfig, deps: MyDeps): MyMethods { * const controller = new MyController(deps.storage, config.logger); - * return controller.getMethods(); + * return controller.makeFacet(); * } * - * getMethods(): MyMethods { + * makeFacet(): MyMethods { * return makeDefaultExo('MyController', { ... }); * } * } @@ -111,13 +111,20 @@ export abstract class Controller< /** * Update state using an immer producer function. - * Persistence is handled automatically by the storage layer. + * State is updated synchronously in memory. + * Persistence is handled automatically by the storage layer (debounced). * * @param producer - Function that mutates a draft of the state. - * @returns Promise that resolves when changes are persisted. */ - protected async update(producer: (draft: State) => void): Promise { - await this.#storage.update(producer); + protected update(producer: (draft: State) => void): void { + this.#storage.update(producer); + } + + /** + * Clear storage and reset to default state. + */ + clearState(): void { + this.#storage.clear(); } /** diff --git a/packages/omnium-gatherum/src/controllers/caplet/caplet-controller.test.ts b/packages/omnium-gatherum/src/controllers/caplet/caplet-controller.test.ts index 7d61afd79..ce483096b 100644 --- a/packages/omnium-gatherum/src/controllers/caplet/caplet-controller.test.ts +++ b/packages/omnium-gatherum/src/controllers/caplet/caplet-controller.test.ts @@ -1,53 +1,26 @@ +import type { Json } from '@metamask/utils'; import { describe, it, expect, vi, beforeEach } from 'vitest'; import { CapletController } from './caplet-controller.ts'; -import type { CapletControllerState } from './caplet-controller.ts'; import type { CapletManifest } from './types.ts'; -import type { ControllerStorage } from '../storage/controller-storage.ts'; +import { makeMockStorageAdapter } from '../../../test/utils.ts'; +import type { StorageAdapter } from '../storage/types.ts'; import type { ControllerConfig } from '../types.ts'; /** - * Create a mock ControllerStorage for testing. - * Maintains in-memory state and tracks update calls. + * Seed a mock adapter with caplet controller state. * - * @param initialState - The initial state for the mock storage. - * @returns A mock ControllerStorage instance with update tracking. + * @param adapter - The adapter to seed. + * @param caplets - The caplets to pre-populate. + * @returns A promise that resolves when seeding is complete. */ -function createMockStorage( - initialState: CapletControllerState, -): ControllerStorage & { updateCalls: (() => void)[] } { - let currentState = { ...initialState }; - const updateCalls: (() => void)[] = []; - - return { - get state(): Readonly { - return harden({ ...currentState }); - }, - - async update( - producer: (draft: CapletControllerState) => void, - ): Promise { - // Create a mutable draft - const draft = JSON.parse( - JSON.stringify(currentState), - ) as CapletControllerState; - producer(draft); - currentState = draft; - updateCalls.push(() => producer(draft)); - }, - - async reload(): Promise { - // No-op for tests - }, - - updateCalls, - }; +async function seedAdapter( + adapter: StorageAdapter, + caplets: Record, +): Promise { + await adapter.set('caplet.caplets', caplets as Json); } -const emptyState: CapletControllerState = { - caplets: {}, -}; - describe('CapletController.make', () => { const mockLogger = { info: vi.fn(), @@ -82,9 +55,9 @@ describe('CapletController.make', () => { describe('install', () => { it('installs a caplet successfully', async () => { - const mockStorage = createMockStorage(emptyState); - const controller = CapletController.make(config, { - storage: mockStorage, + const mockAdapter = makeMockStorageAdapter(); + const controller = await CapletController.make(config, { + adapter: mockAdapter, launchSubcluster: mockLaunchSubcluster, terminateSubcluster: mockTerminateSubcluster, }); @@ -98,9 +71,9 @@ describe('CapletController.make', () => { }); it('validates the manifest', async () => { - const mockStorage = createMockStorage(emptyState); - const controller = CapletController.make(config, { - storage: mockStorage, + const mockAdapter = makeMockStorageAdapter(); + const controller = await CapletController.make(config, { + adapter: mockAdapter, launchSubcluster: mockLaunchSubcluster, terminateSubcluster: mockTerminateSubcluster, }); @@ -113,18 +86,16 @@ describe('CapletController.make', () => { }); it('throws if caplet already installed', async () => { - const stateWithCaplet: CapletControllerState = { - caplets: { - 'com.example.test': { - manifest: validManifest, - subclusterId: 'subcluster-123', - installedAt: 1000, - }, + const mockAdapter = makeMockStorageAdapter(); + await seedAdapter(mockAdapter, { + 'com.example.test': { + manifest: validManifest, + subclusterId: 'subcluster-123', + installedAt: 1000, }, - }; - const mockStorage = createMockStorage(stateWithCaplet); - const controller = CapletController.make(config, { - storage: mockStorage, + }); + const controller = await CapletController.make(config, { + adapter: mockAdapter, launchSubcluster: mockLaunchSubcluster, terminateSubcluster: mockTerminateSubcluster, }); @@ -135,9 +106,9 @@ describe('CapletController.make', () => { }); it('launches subcluster with correct config', async () => { - const mockStorage = createMockStorage(emptyState); - const controller = CapletController.make(config, { - storage: mockStorage, + const mockAdapter = makeMockStorageAdapter(); + const controller = await CapletController.make(config, { + adapter: mockAdapter, launchSubcluster: mockLaunchSubcluster, terminateSubcluster: mockTerminateSubcluster, }); @@ -158,16 +129,16 @@ describe('CapletController.make', () => { vi.useFakeTimers(); vi.setSystemTime(new Date('2024-01-15T12:00:00Z')); - const mockStorage = createMockStorage(emptyState); - const controller = CapletController.make(config, { - storage: mockStorage, + const mockAdapter = makeMockStorageAdapter(); + const controller = await CapletController.make(config, { + adapter: mockAdapter, launchSubcluster: mockLaunchSubcluster, terminateSubcluster: mockTerminateSubcluster, }); await controller.install(validManifest); - const caplet = mockStorage.state.caplets['com.example.test']; + const caplet = await controller.get('com.example.test'); expect(caplet).toBeDefined(); expect(caplet?.manifest).toStrictEqual(validManifest); expect(caplet?.subclusterId).toBe('subcluster-123'); @@ -177,34 +148,31 @@ describe('CapletController.make', () => { }); it('preserves existing caplets when installing', async () => { - const stateWithOtherCaplet: CapletControllerState = { - caplets: { - 'com.other.caplet': { - manifest: { ...validManifest, id: 'com.other.caplet' }, - subclusterId: 'subcluster-other', - installedAt: 500, - }, + const mockAdapter = makeMockStorageAdapter(); + await seedAdapter(mockAdapter, { + 'com.other.caplet': { + manifest: { ...validManifest, id: 'com.other.caplet' }, + subclusterId: 'subcluster-other', + installedAt: 500, }, - }; - const mockStorage = createMockStorage(stateWithOtherCaplet); - const controller = CapletController.make(config, { - storage: mockStorage, + }); + const controller = await CapletController.make(config, { + adapter: mockAdapter, launchSubcluster: mockLaunchSubcluster, terminateSubcluster: mockTerminateSubcluster, }); await controller.install(validManifest); - expect(Object.keys(mockStorage.state.caplets)).toStrictEqual([ - 'com.other.caplet', - 'com.example.test', - ]); + const caplets = await controller.list(); + const capletIds = caplets.map((caplet) => caplet.manifest.id).sort(); + expect(capletIds).toStrictEqual(['com.example.test', 'com.other.caplet']); }); it('logs installation progress', async () => { - const mockStorage = createMockStorage(emptyState); - const controller = CapletController.make(config, { - storage: mockStorage, + const mockAdapter = makeMockStorageAdapter(); + const controller = await CapletController.make(config, { + adapter: mockAdapter, launchSubcluster: mockLaunchSubcluster, terminateSubcluster: mockTerminateSubcluster, }); @@ -222,18 +190,16 @@ describe('CapletController.make', () => { describe('uninstall', () => { it('uninstalls a caplet successfully', async () => { - const stateWithCaplet: CapletControllerState = { - caplets: { - 'com.example.test': { - manifest: validManifest, - subclusterId: 'subcluster-123', - installedAt: 1000, - }, + const mockAdapter = makeMockStorageAdapter(); + await seedAdapter(mockAdapter, { + 'com.example.test': { + manifest: validManifest, + subclusterId: 'subcluster-123', + installedAt: 1000, }, - }; - const mockStorage = createMockStorage(stateWithCaplet); - const controller = CapletController.make(config, { - storage: mockStorage, + }); + const controller = await CapletController.make(config, { + adapter: mockAdapter, launchSubcluster: mockLaunchSubcluster, terminateSubcluster: mockTerminateSubcluster, }); @@ -244,9 +210,9 @@ describe('CapletController.make', () => { }); it('throws if caplet not found', async () => { - const mockStorage = createMockStorage(emptyState); - const controller = CapletController.make(config, { - storage: mockStorage, + const mockAdapter = makeMockStorageAdapter(); + const controller = await CapletController.make(config, { + adapter: mockAdapter, launchSubcluster: mockLaunchSubcluster, terminateSubcluster: mockTerminateSubcluster, }); @@ -257,69 +223,64 @@ describe('CapletController.make', () => { }); it('removes caplet from state', async () => { - const stateWithCaplet: CapletControllerState = { - caplets: { - 'com.example.test': { - manifest: validManifest, - subclusterId: 'subcluster-123', - installedAt: 1000, - }, + const mockAdapter = makeMockStorageAdapter(); + await seedAdapter(mockAdapter, { + 'com.example.test': { + manifest: validManifest, + subclusterId: 'subcluster-123', + installedAt: 1000, }, - }; - const mockStorage = createMockStorage(stateWithCaplet); - const controller = CapletController.make(config, { - storage: mockStorage, + }); + const controller = await CapletController.make(config, { + adapter: mockAdapter, launchSubcluster: mockLaunchSubcluster, terminateSubcluster: mockTerminateSubcluster, }); await controller.uninstall('com.example.test'); - expect(mockStorage.state.caplets['com.example.test']).toBeUndefined(); + const caplet = await controller.get('com.example.test'); + expect(caplet).toBeUndefined(); }); it('preserves other caplets when uninstalling', async () => { - const stateWithCaplets: CapletControllerState = { - caplets: { - 'com.other.caplet': { - manifest: { ...validManifest, id: 'com.other.caplet' }, - subclusterId: 'subcluster-other', - installedAt: 500, - }, - 'com.example.test': { - manifest: validManifest, - subclusterId: 'subcluster-123', - installedAt: 1000, - }, + const mockAdapter = makeMockStorageAdapter(); + await seedAdapter(mockAdapter, { + 'com.other.caplet': { + manifest: { ...validManifest, id: 'com.other.caplet' }, + subclusterId: 'subcluster-other', + installedAt: 500, }, - }; - const mockStorage = createMockStorage(stateWithCaplets); - const controller = CapletController.make(config, { - storage: mockStorage, + 'com.example.test': { + manifest: validManifest, + subclusterId: 'subcluster-123', + installedAt: 1000, + }, + }); + const controller = await CapletController.make(config, { + adapter: mockAdapter, launchSubcluster: mockLaunchSubcluster, terminateSubcluster: mockTerminateSubcluster, }); await controller.uninstall('com.example.test'); - expect(Object.keys(mockStorage.state.caplets)).toStrictEqual([ - 'com.other.caplet', - ]); + const caplets = await controller.list(); + const capletIds = caplets.map((caplet) => caplet.manifest.id); + expect(capletIds).toStrictEqual(['com.other.caplet']); }); it('logs uninstallation progress', async () => { - const stateWithCaplet: CapletControllerState = { - caplets: { - 'com.example.test': { - manifest: validManifest, - subclusterId: 'subcluster-123', - installedAt: 1000, - }, + const mockAdapter = makeMockStorageAdapter(); + await seedAdapter(mockAdapter, { + 'com.example.test': { + manifest: validManifest, + subclusterId: 'subcluster-123', + installedAt: 1000, }, - }; - const mockStorage = createMockStorage(stateWithCaplet); - const controller = CapletController.make(config, { - storage: mockStorage, + }); + const controller = await CapletController.make(config, { + adapter: mockAdapter, launchSubcluster: mockLaunchSubcluster, terminateSubcluster: mockTerminateSubcluster, }); @@ -337,9 +298,9 @@ describe('CapletController.make', () => { describe('list', () => { it('returns empty array when no caplets installed', async () => { - const mockStorage = createMockStorage(emptyState); - const controller = CapletController.make(config, { - storage: mockStorage, + const mockAdapter = makeMockStorageAdapter(); + const controller = await CapletController.make(config, { + adapter: mockAdapter, launchSubcluster: mockLaunchSubcluster, terminateSubcluster: mockTerminateSubcluster, }); @@ -355,23 +316,21 @@ describe('CapletController.make', () => { id: 'com.example.test2', name: 'Test Caplet 2', }; - const stateWithCaplets: CapletControllerState = { - caplets: { - 'com.example.test': { - manifest: validManifest, - subclusterId: 'subcluster-1', - installedAt: 1000, - }, - 'com.example.test2': { - manifest: manifest2, - subclusterId: 'subcluster-2', - installedAt: 2000, - }, + const mockAdapter = makeMockStorageAdapter(); + await seedAdapter(mockAdapter, { + 'com.example.test': { + manifest: validManifest, + subclusterId: 'subcluster-1', + installedAt: 1000, }, - }; - const mockStorage = createMockStorage(stateWithCaplets); - const controller = CapletController.make(config, { - storage: mockStorage, + 'com.example.test2': { + manifest: manifest2, + subclusterId: 'subcluster-2', + installedAt: 2000, + }, + }); + const controller = await CapletController.make(config, { + adapter: mockAdapter, launchSubcluster: mockLaunchSubcluster, terminateSubcluster: mockTerminateSubcluster, }); @@ -394,18 +353,16 @@ describe('CapletController.make', () => { describe('get', () => { it('returns caplet if exists', async () => { - const stateWithCaplet: CapletControllerState = { - caplets: { - 'com.example.test': { - manifest: validManifest, - subclusterId: 'subcluster-123', - installedAt: 1705320000000, - }, + const mockAdapter = makeMockStorageAdapter(); + await seedAdapter(mockAdapter, { + 'com.example.test': { + manifest: validManifest, + subclusterId: 'subcluster-123', + installedAt: 1705320000000, }, - }; - const mockStorage = createMockStorage(stateWithCaplet); - const controller = CapletController.make(config, { - storage: mockStorage, + }); + const controller = await CapletController.make(config, { + adapter: mockAdapter, launchSubcluster: mockLaunchSubcluster, terminateSubcluster: mockTerminateSubcluster, }); @@ -420,9 +377,9 @@ describe('CapletController.make', () => { }); it('returns undefined if caplet not found', async () => { - const mockStorage = createMockStorage(emptyState); - const controller = CapletController.make(config, { - storage: mockStorage, + const mockAdapter = makeMockStorageAdapter(); + const controller = await CapletController.make(config, { + adapter: mockAdapter, launchSubcluster: mockLaunchSubcluster, terminateSubcluster: mockTerminateSubcluster, }); @@ -435,18 +392,16 @@ describe('CapletController.make', () => { describe('getByService', () => { it('returns caplet providing the service', async () => { - const stateWithCaplet: CapletControllerState = { - caplets: { - 'com.example.test': { - manifest: validManifest, - subclusterId: 'subcluster-123', - installedAt: 1000, - }, + const mockAdapter = makeMockStorageAdapter(); + await seedAdapter(mockAdapter, { + 'com.example.test': { + manifest: validManifest, + subclusterId: 'subcluster-123', + installedAt: 1000, }, - }; - const mockStorage = createMockStorage(stateWithCaplet); - const controller = CapletController.make(config, { - storage: mockStorage, + }); + const controller = await CapletController.make(config, { + adapter: mockAdapter, launchSubcluster: mockLaunchSubcluster, terminateSubcluster: mockTerminateSubcluster, }); @@ -458,18 +413,16 @@ describe('CapletController.make', () => { }); it('returns undefined if no caplet provides the service', async () => { - const stateWithCaplet: CapletControllerState = { - caplets: { - 'com.example.test': { - manifest: validManifest, - subclusterId: 'subcluster-123', - installedAt: 1000, - }, + const mockAdapter = makeMockStorageAdapter(); + await seedAdapter(mockAdapter, { + 'com.example.test': { + manifest: validManifest, + subclusterId: 'subcluster-123', + installedAt: 1000, }, - }; - const mockStorage = createMockStorage(stateWithCaplet); - const controller = CapletController.make(config, { - storage: mockStorage, + }); + const controller = await CapletController.make(config, { + adapter: mockAdapter, launchSubcluster: mockLaunchSubcluster, terminateSubcluster: mockTerminateSubcluster, }); @@ -486,23 +439,21 @@ describe('CapletController.make', () => { name: 'Test Caplet 2', providedServices: ['signer', 'verifier'], }; - const stateWithCaplets: CapletControllerState = { - caplets: { - 'com.example.test': { - manifest: validManifest, - subclusterId: 'subcluster-1', - installedAt: 1000, - }, - 'com.example.test2': { - manifest: manifest2, - subclusterId: 'subcluster-2', - installedAt: 2000, - }, + const mockAdapter = makeMockStorageAdapter(); + await seedAdapter(mockAdapter, { + 'com.example.test': { + manifest: validManifest, + subclusterId: 'subcluster-1', + installedAt: 1000, }, - }; - const mockStorage = createMockStorage(stateWithCaplets); - const controller = CapletController.make(config, { - storage: mockStorage, + 'com.example.test2': { + manifest: manifest2, + subclusterId: 'subcluster-2', + installedAt: 2000, + }, + }); + const controller = await CapletController.make(config, { + adapter: mockAdapter, launchSubcluster: mockLaunchSubcluster, terminateSubcluster: mockTerminateSubcluster, }); diff --git a/packages/omnium-gatherum/src/controllers/caplet/caplet-controller.ts b/packages/omnium-gatherum/src/controllers/caplet/caplet-controller.ts index 584694631..3f7a062d4 100644 --- a/packages/omnium-gatherum/src/controllers/caplet/caplet-controller.ts +++ b/packages/omnium-gatherum/src/controllers/caplet/caplet-controller.ts @@ -12,7 +12,8 @@ import type { import { isCapletManifest } from './types.ts'; import { Controller } from '../base-controller.ts'; import type { ControllerConfig } from '../base-controller.ts'; -import type { ControllerStorage } from '../storage/controller-storage.ts'; +import { ControllerStorage } from '../storage/controller-storage.ts'; +import type { StorageAdapter } from '../storage/types.ts'; /** * Caplet controller persistent state. @@ -76,8 +77,8 @@ export type CapletControllerFacet = { * These are attenuated - only the methods needed are provided. */ export type CapletControllerDeps = { - /** State storage for caplet data */ - storage: ControllerStorage; + /** Storage adapter for creating controller storage */ + adapter: StorageAdapter; /** Launch a subcluster for a caplet */ launchSubcluster: (config: ClusterConfig) => Promise; /** Terminate a caplet's subcluster */ @@ -129,12 +130,20 @@ export class CapletController extends Controller< * @param deps - Controller dependencies (attenuated for POLA). * @returns A hardened CapletController exo. */ - static make( + static async make( config: ControllerConfig, deps: CapletControllerDeps, - ): CapletControllerFacet { + ): Promise { + // Create storage internally + const storage = await ControllerStorage.make({ + namespace: 'caplet', + adapter: deps.adapter, + defaultState: { caplets: {} }, + logger: config.logger.subLogger({ tags: ['storage'] }), + }); + const controller = new CapletController( - deps.storage, + storage, config.logger, deps.launchSubcluster, deps.terminateSubcluster, @@ -209,8 +218,7 @@ export class CapletController extends Controller< // Launch subcluster const { subclusterId } = await this.#launchSubcluster(clusterConfig); - // Store caplet data - await this.update((draft) => { + this.update((draft) => { draft.caplets[id] = { manifest, subclusterId, @@ -238,8 +246,7 @@ export class CapletController extends Controller< // Terminate the subcluster await this.#terminateSubcluster(caplet.subclusterId); - // Remove from storage - await this.update((draft) => { + this.update((draft) => { delete draft.caplets[capletId]; }); diff --git a/packages/omnium-gatherum/src/controllers/index.ts b/packages/omnium-gatherum/src/controllers/index.ts index cc6326308..120d56561 100644 --- a/packages/omnium-gatherum/src/controllers/index.ts +++ b/packages/omnium-gatherum/src/controllers/index.ts @@ -7,13 +7,11 @@ export { makeFacet } from './facet.ts'; export type { NamespacedStorage, StorageAdapter, - ControllerStorage, ControllerStorageConfig, } from './storage/index.ts'; export { makeChromeStorageAdapter, - makeNamespacedStorage, - makeControllerStorage, + ControllerStorage, } from './storage/index.ts'; // Caplet diff --git a/packages/omnium-gatherum/src/controllers/storage/controller-storage.test.ts b/packages/omnium-gatherum/src/controllers/storage/controller-storage.test.ts index c01093441..93ea2b5c2 100644 --- a/packages/omnium-gatherum/src/controllers/storage/controller-storage.test.ts +++ b/packages/omnium-gatherum/src/controllers/storage/controller-storage.test.ts @@ -1,6 +1,6 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; -import { makeControllerStorage } from './controller-storage.ts'; +import { ControllerStorage } from './controller-storage.ts'; import type { StorageAdapter } from './types.ts'; type TestState = { @@ -9,7 +9,7 @@ type TestState = { count: number; }; -describe('makeControllerStorage', () => { +describe('ControllerStorage', () => { const mockAdapter: StorageAdapter = { get: vi.fn(), set: vi.fn(), @@ -17,6 +17,14 @@ describe('makeControllerStorage', () => { keys: vi.fn(), }; + const mockLogger = { + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + debug: vi.fn(), + subLogger: vi.fn().mockReturnThis(), + }; + const defaultState: TestState = { installed: [], manifests: {}, @@ -24,6 +32,7 @@ describe('makeControllerStorage', () => { }; beforeEach(() => { + vi.clearAllMocks(); vi.mocked(mockAdapter.get).mockResolvedValue(undefined); vi.mocked(mockAdapter.set).mockResolvedValue(undefined); vi.mocked(mockAdapter.delete).mockResolvedValue(undefined); @@ -46,10 +55,12 @@ describe('makeControllerStorage', () => { return undefined; }); - const storage = await makeControllerStorage({ + const storage = await ControllerStorage.make({ namespace: 'test', adapter: mockAdapter, defaultState, + logger: mockLogger as never, + debounceMs: 0, }); expect(storage.state.installed).toStrictEqual(['app1']); @@ -67,7 +78,7 @@ describe('makeControllerStorage', () => { return undefined; }); - const storage = await makeControllerStorage({ + const storage = await ControllerStorage.make({ namespace: 'test', adapter: mockAdapter, defaultState: { @@ -75,6 +86,8 @@ describe('makeControllerStorage', () => { manifests: {}, metadata: { version: 1 }, }, + logger: mockLogger as never, + debounceMs: 0, }); expect(storage.state.installed).toStrictEqual(['existing']); @@ -85,10 +98,12 @@ describe('makeControllerStorage', () => { it('uses all defaults when storage is empty', async () => { vi.mocked(mockAdapter.keys).mockResolvedValue([]); - const storage = await makeControllerStorage({ + const storage = await ControllerStorage.make({ namespace: 'test', adapter: mockAdapter, defaultState, + logger: mockLogger as never, + debounceMs: 0, }); expect(storage.state.installed).toStrictEqual([]); @@ -97,10 +112,12 @@ describe('makeControllerStorage', () => { }); it('returns hardened state copy', async () => { - const storage = await makeControllerStorage({ + const storage = await ControllerStorage.make({ namespace: 'test', adapter: mockAdapter, defaultState: { items: ['original'] as string[] }, + logger: mockLogger as never, + debounceMs: 0, }); // Get a reference to the state @@ -125,10 +142,12 @@ describe('makeControllerStorage', () => { vi.mocked(mockAdapter.keys).mockResolvedValue(['ns.count']); vi.mocked(mockAdapter.get).mockResolvedValue(42); - const storage = await makeControllerStorage({ + const storage = await ControllerStorage.make({ namespace: 'ns', adapter: mockAdapter, defaultState: { count: 0 }, + logger: mockLogger as never, + debounceMs: 0, }); expect(storage.state.count).toBe(42); @@ -137,99 +156,135 @@ describe('makeControllerStorage', () => { describe('update', () => { it('persists only modified top-level keys', async () => { - const storage = await makeControllerStorage({ + const storage = await ControllerStorage.make({ namespace: 'test', adapter: mockAdapter, defaultState, + logger: mockLogger as never, + debounceMs: 0, }); - await storage.update((draft) => { + storage.update((draft) => { draft.installed.push('new-app'); // manifests and count not modified }); + // Wait for persistence (debounced but set to 0ms) + await new Promise((resolve) => setTimeout(resolve, 10)); + expect(mockAdapter.set).toHaveBeenCalledTimes(1); expect(mockAdapter.set).toHaveBeenCalledWith('test.installed', [ 'new-app', ]); }); - it('updates in-memory state after persistence', async () => { - const storage = await makeControllerStorage({ + it('updates in-memory state immediately', async () => { + const storage = await ControllerStorage.make({ namespace: 'test', adapter: mockAdapter, defaultState, + logger: mockLogger as never, + debounceMs: 0, }); - await storage.update((draft) => { + storage.update((draft) => { draft.installed.push('item1'); }); + // State updated synchronously expect(storage.state.installed).toStrictEqual(['item1']); }); it('does not persist when no changes made', async () => { - const storage = await makeControllerStorage({ + // Clear any pending operations from previous tests + await new Promise((resolve) => setTimeout(resolve, 15)); + vi.clearAllMocks(); + + const storage = await ControllerStorage.make({ namespace: 'test', adapter: mockAdapter, defaultState, + logger: mockLogger as never, + debounceMs: 0, }); - await storage.update((draft) => { + storage.update((draft) => { // No actual changes draft.count = 0; }); + // Wait for potential persistence + await new Promise((resolve) => setTimeout(resolve, 10)); + expect(mockAdapter.set).not.toHaveBeenCalled(); }); it('persists multiple modified keys', async () => { - const storage = await makeControllerStorage({ + const storage = await ControllerStorage.make({ namespace: 'test', adapter: mockAdapter, defaultState: { a: 1, b: 2, c: 3 }, + logger: mockLogger as never, + debounceMs: 0, }); - await storage.update((draft) => { + storage.update((draft) => { draft.a = 10; draft.c = 30; }); + // Wait for persistence + await new Promise((resolve) => setTimeout(resolve, 10)); + expect(mockAdapter.set).toHaveBeenCalledTimes(2); expect(mockAdapter.set).toHaveBeenCalledWith('test.a', 10); expect(mockAdapter.set).toHaveBeenCalledWith('test.c', 30); }); - it('does not update state if persistence fails', async () => { + it('updates state even if persistence fails (fire-and-forget)', async () => { vi.mocked(mockAdapter.set).mockRejectedValue(new Error('Storage error')); - const storage = await makeControllerStorage({ + const storage = await ControllerStorage.make({ namespace: 'test', adapter: mockAdapter, defaultState, + logger: mockLogger as never, + debounceMs: 0, }); - await expect( - storage.update((draft) => { - draft.count = 100; - }), - ).rejects.toThrow('Storage error'); + storage.update((draft) => { + draft.count = 100; + }); - // State should remain unchanged - expect(storage.state.count).toBe(0); + // State updated immediately despite persistence failure + expect(storage.state.count).toBe(100); + + // Wait for persistence attempt + await new Promise((resolve) => setTimeout(resolve, 10)); + + // Error should be logged + expect(mockLogger.error).toHaveBeenCalledWith( + 'Failed to persist state changes:', + expect.any(Error), + ); }); it('handles nested object modifications', async () => { - const storage = await makeControllerStorage({ + const storage = await ControllerStorage.make({ namespace: 'test', adapter: mockAdapter, defaultState, + logger: mockLogger as never, + debounceMs: 0, }); - await storage.update((draft) => { + storage.update((draft) => { draft.manifests['new-app'] = { name: 'New App' }; }); + // Wait for persistence + await new Promise((resolve) => setTimeout(resolve, 10)); + expect(mockAdapter.set).toHaveBeenCalledWith('test.manifests', { 'new-app': { name: 'New App' }, }); @@ -239,16 +294,21 @@ describe('makeControllerStorage', () => { vi.mocked(mockAdapter.keys).mockResolvedValue(['test.installed']); vi.mocked(mockAdapter.get).mockResolvedValue(['app1', 'app2']); - const storage = await makeControllerStorage({ + const storage = await ControllerStorage.make({ namespace: 'test', adapter: mockAdapter, defaultState, + logger: mockLogger as never, + debounceMs: 0, }); - await storage.update((draft) => { + storage.update((draft) => { draft.installed = draft.installed.filter((id) => id !== 'app1'); }); + // Wait for persistence + await new Promise((resolve) => setTimeout(resolve, 10)); + expect(mockAdapter.set).toHaveBeenCalledWith('test.installed', ['app2']); }); @@ -259,78 +319,195 @@ describe('makeControllerStorage', () => { app2: { name: 'App 2' }, }); - const storage = await makeControllerStorage({ + const storage = await ControllerStorage.make({ namespace: 'test', adapter: mockAdapter, defaultState, + logger: mockLogger as never, + debounceMs: 0, }); - await storage.update((draft) => { + storage.update((draft) => { delete draft.manifests.app1; }); + // Wait for persistence + await new Promise((resolve) => setTimeout(resolve, 10)); + expect(mockAdapter.set).toHaveBeenCalledWith('test.manifests', { app2: { name: 'App 2' }, }); }); }); - describe('reload', () => { - it('reloads state from storage', async () => { - vi.mocked(mockAdapter.keys).mockResolvedValue([]); + describe('namespace isolation', () => { + it('uses different prefixes for different namespaces', async () => { + await ControllerStorage.make({ + namespace: 'caplet', + adapter: mockAdapter, + defaultState: { value: 1 }, + logger: mockLogger as never, + debounceMs: 0, + }); - const storage = await makeControllerStorage({ + await ControllerStorage.make({ + namespace: 'service', + adapter: mockAdapter, + defaultState: { value: 2 }, + logger: mockLogger as never, + debounceMs: 0, + }); + + expect(mockAdapter.keys).toHaveBeenCalledWith('caplet.'); + expect(mockAdapter.keys).toHaveBeenCalledWith('service.'); + }); + }); + + describe('debouncing with key accumulation', () => { + it('accumulates modified keys across multiple updates', async () => { + vi.useFakeTimers(); + const storage = await ControllerStorage.make({ namespace: 'test', adapter: mockAdapter, - defaultState, + defaultState: { a: 0, b: 0, c: 0 }, + logger: mockLogger as never, + debounceMs: 100, }); - expect(storage.state.count).toBe(0); + // First update: modifies a and b + storage.update((draft) => { + draft.a = 1; + draft.b = 1; + }); + + // Second update at t=50ms: modifies only a + vi.advanceTimersByTime(50); + storage.update((draft) => { + draft.a = 2; + }); - // Simulate external storage update - vi.mocked(mockAdapter.keys).mockResolvedValue(['test.count']); - vi.mocked(mockAdapter.get).mockResolvedValue(999); + // Timer should fire at t=100ms (from first update) + vi.advanceTimersByTime(50); + await vi.runAllTimersAsync(); - await storage.reload(); + // Both a and b should be persisted (accumulated keys) + expect(mockAdapter.set).toHaveBeenCalledWith('test.a', 2); + expect(mockAdapter.set).toHaveBeenCalledWith('test.b', 1); - expect(storage.state.count).toBe(999); + vi.useRealTimers(); }); - it('merges with defaults after reload', async () => { - vi.mocked(mockAdapter.keys).mockResolvedValue(['test.count']); - vi.mocked(mockAdapter.get).mockResolvedValue(42); + it('does not reset timer on subsequent writes', async () => { + vi.useFakeTimers(); + const storage = await ControllerStorage.make({ + namespace: 'test', + adapter: mockAdapter, + defaultState: { a: 0 }, + logger: mockLogger as never, + debounceMs: 100, + }); - const storage = await makeControllerStorage({ + storage.update((draft) => { + draft.a = 1; + }); + + // Second write at t=90ms (before first timer fires) + vi.advanceTimersByTime(90); + storage.update((draft) => { + draft.a = 2; + }); + + // Timer fires at t=100ms (NOT reset to t=190ms) + vi.advanceTimersByTime(10); + await vi.runAllTimersAsync(); + + expect(mockAdapter.set).toHaveBeenCalledWith('test.a', 2); + + vi.useRealTimers(); + }); + + it('writes immediately when idle > debounceMs', async () => { + vi.useFakeTimers(); + const storage = await ControllerStorage.make({ namespace: 'test', adapter: mockAdapter, - defaultState, + defaultState: { a: 0 }, + logger: mockLogger as never, + debounceMs: 100, + }); + + storage.update((draft) => { + draft.a = 1; }); + await vi.runAllTimersAsync(); + vi.clearAllMocks(); - // Reload - count from storage, others from defaults - await storage.reload(); + // Wait 150ms (> debounceMs) + vi.advanceTimersByTime(150); - expect(storage.state.count).toBe(42); - expect(storage.state.installed).toStrictEqual([]); - expect(storage.state.manifests).toStrictEqual({}); + // Next write should be immediate (no debounce) + storage.update((draft) => { + draft.a = 2; + }); + await vi.runAllTimersAsync(); + + expect(mockAdapter.set).toHaveBeenCalledWith('test.a', 2); + + vi.useRealTimers(); }); }); - describe('namespace isolation', () => { - it('uses different prefixes for different namespaces', async () => { - await makeControllerStorage({ - namespace: 'caplet', + describe('clear', () => { + it('resets state to default', async () => { + const testDefaultState = { items: [] as string[], count: 0 }; + const storage = await ControllerStorage.make({ + namespace: 'test', adapter: mockAdapter, - defaultState: { value: 1 }, + defaultState: testDefaultState, + logger: mockLogger as never, + debounceMs: 0, }); - await makeControllerStorage({ - namespace: 'service', + // Modify state + storage.update((draft) => { + draft.items.push('item1'); + draft.count = 1; + }); + + expect(storage.state.items).toStrictEqual(['item1']); + expect(storage.state.count).toBe(1); + + // Clear + storage.clear(); + + expect(storage.state.items).toStrictEqual([]); + expect(storage.state.count).toBe(0); + }); + + it('persists cleared state', async () => { + const clearDefaultState = { a: 0, b: 0 }; + const storage = await ControllerStorage.make({ + namespace: 'test', adapter: mockAdapter, - defaultState: { value: 2 }, + defaultState: clearDefaultState, + logger: mockLogger as never, + debounceMs: 0, }); - expect(mockAdapter.keys).toHaveBeenCalledWith('caplet.'); - expect(mockAdapter.keys).toHaveBeenCalledWith('service.'); + storage.update((draft) => { + draft.a = 5; + draft.b = 10; + }); + + await new Promise((resolve) => setTimeout(resolve, 10)); + vi.clearAllMocks(); + + storage.clear(); + + await new Promise((resolve) => setTimeout(resolve, 10)); + + expect(mockAdapter.set).toHaveBeenCalledWith('test.a', 0); + expect(mockAdapter.set).toHaveBeenCalledWith('test.b', 0); }); }); }); diff --git a/packages/omnium-gatherum/src/controllers/storage/controller-storage.ts b/packages/omnium-gatherum/src/controllers/storage/controller-storage.ts index 22ef617f7..a2c1939e9 100644 --- a/packages/omnium-gatherum/src/controllers/storage/controller-storage.ts +++ b/packages/omnium-gatherum/src/controllers/storage/controller-storage.ts @@ -1,3 +1,4 @@ +import type { Logger } from '@metamask/logger'; import type { Json } from '@metamask/utils'; import { enablePatches, produce } from 'immer'; import type { Patch } from 'immer'; @@ -21,108 +22,121 @@ export type ControllerStorageConfig> = { adapter: StorageAdapter; /** Default state values - used for initialization and type inference */ defaultState: State; + /** Logger for storage operations */ + logger: Logger; + /** Debounce delay in milliseconds (default: 100, set to 0 for tests) */ + debounceMs?: number; }; +/** + * Internal options passed to constructor after async initialization. + */ +type ControllerStorageOptions> = + ControllerStorageConfig & { + /** Initial state loaded from storage */ + initialState: State; + }; + /** * ControllerStorage provides a simplified state management interface for controllers. * * Features: * - Flat top-level key mapping: `state.foo` maps to `{namespace}.foo` in storage * - Immer-based updates with automatic change detection + * - Synchronous state updates with debounced persistence * - Only modified top-level keys are persisted + * - Fire-and-forget persistence (errors logged but don't rollback state) * - Eager loading on initialization * * @template State - The state object type (must have Json-serializable values) */ -export type ControllerStorage> = { +export class ControllerStorage> { + readonly #adapter: StorageAdapter; + + readonly #prefix: string; + + readonly #defaultState: State; + + readonly #logger: Logger; + + readonly #debounceMs: number; + + #state: State; + + #pendingPersist: ReturnType | null = null; + + readonly #pendingKeys: Set = new Set(); + + #lastWriteTime: number = 0; + /** - * Current state (readonly, hardened). - * Access individual properties: `storage.state.installed` + * Private constructor - use static make() factory method. + * + * @param options - Configuration including initial loaded state. */ - readonly state: Readonly; + // eslint-disable-next-line no-restricted-syntax -- TypeScript doesn't support # for constructors + private constructor(options: ControllerStorageOptions) { + this.#adapter = options.adapter; + this.#prefix = `${options.namespace}.`; + this.#defaultState = options.defaultState; + this.#logger = options.logger; + this.#debounceMs = options.debounceMs ?? 100; + this.#state = options.initialState; + } /** - * Update state using an immer producer function. - * Only modified top-level keys will be persisted to storage. + * Create a ControllerStorage instance for a controller. * - * @param producer - Function that mutates a draft of the state - * @returns Promise that resolves when changes are persisted - * @throws If storage persistence fails (state remains unchanged) + * This factory function: + * 1. Loads existing state from storage for the namespace + * 2. Merges with defaults (storage values take precedence) + * 3. Returns a hardened ControllerStorage instance + * + * @param config - Configuration including namespace, adapter, and default state. + * @returns Promise resolving to a hardened ControllerStorage instance. * * @example * ```typescript - * await storage.update(draft => { - * draft.installed.push('com.example.app'); - * draft.manifests['com.example.app'] = manifest; + * const capletState = await ControllerStorage.make({ + * namespace: 'caplet', + * adapter: storageAdapter, + * defaultState: { installed: [], manifests: {} }, + * logger: logger.subLogger({ tags: ['storage'] }), * }); - * ``` - */ - update: (producer: (draft: State) => void) => Promise; - - /** - * Force reload state from storage. - * Useful for syncing after external storage changes. - */ - reload: () => Promise; -}; - -/** - * Create a ControllerStorage instance for a controller. - * - * This factory function: - * 1. Loads existing state from storage for the namespace - * 2. Merges with defaults (storage values take precedence) - * 3. Returns a hardened ControllerStorage interface - * - * @param config - Configuration including namespace, adapter, and default state. - * @returns Promise resolving to a hardened ControllerStorage instance. - * - * @example - * ```typescript - * const capletState = await makeControllerStorage({ - * namespace: 'caplet', - * adapter: storageAdapter, - * defaultState: { installed: [], manifests: {} } - * }); - * - * // Read state - * console.log(capletState.state.installed); - * - * // Update state - * await capletState.update(draft => { - * draft.installed.push('com.example.app'); - * }); - * ``` - */ -export async function makeControllerStorage>( - config: ControllerStorageConfig, -): Promise> { - const { namespace, adapter, defaultState } = config; - const prefix = `${namespace}.`; - - /** - * Build a storage key from a state property name. * - * @param stateKey - The state property name. - * @returns The namespaced storage key. - */ - const buildKey = (stateKey: string): string => `${prefix}${stateKey}`; - - /** - * Strip namespace prefix from a storage key. + * // Read state + * console.log(capletState.state.installed); * - * @param fullKey - The full namespaced storage key. - * @returns The state property name without prefix. + * // Update state (synchronous) + * capletState.update(draft => { + * draft.installed.push('com.example.app'); + * }); + * ``` */ - const stripPrefix = (fullKey: string): string => fullKey.slice(prefix.length); + static async make>( + config: ControllerStorageConfig, + ): Promise> { + const initialState = await this.#loadState(config); + return harden( + new ControllerStorage({ + ...config, + initialState, + }), + ); + } /** * Load all state from storage, merging with defaults. * Storage values take precedence over defaults. * + * @param config - Configuration with adapter, namespace, and defaults. * @returns The merged state object. */ - const loadState = async (): Promise => { + static async #loadState>( + config: ControllerStorageConfig, + ): Promise { + const { namespace, adapter, defaultState } = config; + const prefix = `${namespace}.`; const allKeys = await adapter.keys(prefix); // Start with a copy of defaults @@ -131,7 +145,7 @@ export async function makeControllerStorage>( // Load and merge values from storage await Promise.all( allKeys.map(async (fullKey) => { - const key = stripPrefix(fullKey) as keyof State; + const key = fullKey.slice(prefix.length) as keyof State; const value = await adapter.get(fullKey); if (value !== undefined) { state[key] = value as State[keyof State]; @@ -142,26 +156,136 @@ export async function makeControllerStorage>( return produce({}, (draft) => { Object.assign(draft, state); }) as State; - }; + } + + /** + * Current state (readonly, deeply frozen by immer). + * Access individual properties: `storage.state.installed` + * + * @returns The current readonly state. + */ + get state(): Readonly { + return this.#state; + } + + /** + * Update state using an immer producer function. + * State is updated synchronously in memory. + * Persistence is queued and debounced (fire-and-forget). + * + * @param producer - Function that mutates a draft of the state or returns new state + * + * @example + * ```typescript + * // Mutate draft + * storage.update(draft => { + * draft.installed.push('com.example.app'); + * draft.manifests['com.example.app'] = manifest; + * }); + */ + update(producer: (draft: State) => void | State): void { + // Capture state before operations to avoid race conditions + const stateSnapshot = this.#state; + + // Use immer's produce with patches callback to track changes + let patches: Patch[] = []; + const nextState = produce(stateSnapshot, producer, (patchList) => { + patches = patchList; + }); + + // No changes - nothing to do + if (patches.length === 0) { + return; + } + + // Update in-memory state immediately + this.#state = nextState; + + // Queue debounced persistence (fire-and-forget) + this.#schedulePersist(patches); + } + + /** + * Clear all state and reset to default values. + * Updates state synchronously, persistence is debounced. + */ + clear(): void { + this.update((draft) => { + Object.assign(draft, this.#defaultState); + }); + } /** - * Persist specific keys to storage. + * Schedule debounced persistence with key accumulation. + * Implements bounded latency (timer not reset) and immediate writes after idle. * - * @param stateToSave - The state object containing values to persist. + * @param patches - Immer patches describing changes. + */ + #schedulePersist(patches: Patch[]): void { + const now = Date.now(); + const timeSinceLastWrite = now - this.#lastWriteTime; + this.#lastWriteTime = now; + + const modifiedKeys = this.#getModifiedKeys(patches); + for (const key of modifiedKeys) { + this.#pendingKeys.add(key); + } + + if ( + timeSinceLastWrite > this.#debounceMs && + this.#pendingPersist === null + ) { + this.#flushPendingWrites(); + return; + } + + if (this.#pendingPersist === null) { + this.#pendingPersist = setTimeout(() => { + this.#flushPendingWrites(); + }, this.#debounceMs); + } + // else: timer already running, just accumulate keys, don't reset + } + + /** + * Flush pending writes to storage. + * Captures accumulated keys and persists current state values. + */ + #flushPendingWrites(): void { + if (this.#pendingKeys.size === 0) { + this.#pendingPersist = null; + return; + } + + const keysToWrite = new Set(this.#pendingKeys); + this.#pendingKeys.clear(); + this.#pendingPersist = null; + + // Persist current state values for accumulated keys + this.#persistAccumulatedKeys(this.#state, keysToWrite).catch((error) => { + this.#logger.error('Failed to persist state changes:', error); + }); + } + + /** + * Persist accumulated keys to storage. + * Always persists current state values (last-write-wins). + * + * @param state - The current state to persist from. * @param keys - Set of top-level keys to persist. */ - const persistKeys = async ( - stateToSave: State, + async #persistAccumulatedKeys( + state: State, keys: Set, - ): Promise => { + ): Promise { await Promise.all( Array.from(keys).map(async (key) => { - const storageKey = buildKey(key); - const value = stateToSave[key as keyof State]; - await adapter.set(storageKey, value as Json); + const storageKey = this.#buildKey(key); + const value = state[key as keyof State]; + await this.#adapter.set(storageKey, value as Json); }), ); - }; + } /** * Extract top-level keys that were modified from immer patches. @@ -169,7 +293,7 @@ export async function makeControllerStorage>( * @param patches - Array of immer patches describing changes. * @returns Set of modified top-level keys. */ - const getModifiedKeys = (patches: Patch[]): Set => { + #getModifiedKeys(patches: Patch[]): Set { const keys = new Set(); for (const patch of patches) { // The first element of path is always the top-level key @@ -178,47 +302,16 @@ export async function makeControllerStorage>( } } return keys; - }; - - // Load initial state - let currentState = await loadState(); - - const storage: ControllerStorage = { - get state(): Readonly { - return currentState; - }, - - async update(producer: (draft: State) => void): Promise { - // Capture state before async operations to avoid race conditions - const stateSnapshot = currentState; - - // Use immer's produce with patches callback to track changes - let patches: Patch[] = []; - const nextState = produce(stateSnapshot, producer, (patchList) => { - patches = patchList; - }); + } - // No changes - nothing to do - if (patches.length === 0) { - return; - } - - // Determine which top-level keys changed - const modifiedKeys = getModifiedKeys(patches); - - // Persist only the modified keys - await persistKeys(nextState, modifiedKeys); - - // Update in-memory state only after successful persistence - // eslint-disable-next-line require-atomic-updates -- Last-write-wins is intentional - currentState = nextState; - }, - - async reload(): Promise { - currentState = await loadState(); - }, - }; - - return harden(storage); + /** + * Build a storage key from a state property name. + * + * @param stateKey - The state property name. + * @returns The namespaced storage key. + */ + #buildKey(stateKey: string): string { + return `${this.#prefix}${stateKey}`; + } } -harden(makeControllerStorage); +harden(ControllerStorage); diff --git a/packages/omnium-gatherum/src/controllers/storage/index.ts b/packages/omnium-gatherum/src/controllers/storage/index.ts index 9a88b8acf..8f0382e45 100644 --- a/packages/omnium-gatherum/src/controllers/storage/index.ts +++ b/packages/omnium-gatherum/src/controllers/storage/index.ts @@ -1,8 +1,4 @@ export type { NamespacedStorage, StorageAdapter } from './types.ts'; -export type { - ControllerStorage, - ControllerStorageConfig, -} from './controller-storage.ts'; +export type { ControllerStorageConfig } from './controller-storage.ts'; export { makeChromeStorageAdapter } from './chrome-storage.ts'; -export { makeNamespacedStorage } from './namespaced-storage.ts'; -export { makeControllerStorage } from './controller-storage.ts'; +export { ControllerStorage } from './controller-storage.ts'; diff --git a/packages/omnium-gatherum/src/controllers/storage/namespaced-storage.test.ts b/packages/omnium-gatherum/src/controllers/storage/namespaced-storage.test.ts deleted file mode 100644 index b427b63fe..000000000 --- a/packages/omnium-gatherum/src/controllers/storage/namespaced-storage.test.ts +++ /dev/null @@ -1,156 +0,0 @@ -import { describe, it, expect, vi, beforeEach } from 'vitest'; - -import { makeNamespacedStorage } from './namespaced-storage.ts'; -import type { StorageAdapter } from './types.ts'; - -describe('makeNamespacedStorage', () => { - const mockAdapter: StorageAdapter = { - get: vi.fn(), - set: vi.fn(), - delete: vi.fn(), - keys: vi.fn(), - }; - - beforeEach(() => { - vi.clearAllMocks(); - vi.mocked(mockAdapter.get).mockResolvedValue(undefined); - vi.mocked(mockAdapter.set).mockResolvedValue(undefined); - vi.mocked(mockAdapter.delete).mockResolvedValue(undefined); - vi.mocked(mockAdapter.keys).mockResolvedValue([]); - }); - - describe('get', () => { - it('prefixes key with namespace', async () => { - vi.mocked(mockAdapter.get).mockResolvedValue('value'); - - const storage = makeNamespacedStorage('caplet', mockAdapter); - const result = await storage.get('myKey'); - - expect(result).toBe('value'); - expect(mockAdapter.get).toHaveBeenCalledWith('caplet.myKey'); - }); - - it('returns undefined for non-existent key', async () => { - vi.mocked(mockAdapter.get).mockResolvedValue(undefined); - - const storage = makeNamespacedStorage('caplet', mockAdapter); - const result = await storage.get('nonExistent'); - - expect(result).toBeUndefined(); - }); - }); - - describe('set', () => { - it('prefixes key with namespace', async () => { - const storage = makeNamespacedStorage('caplet', mockAdapter); - await storage.set('myKey', 'myValue'); - - expect(mockAdapter.set).toHaveBeenCalledWith('caplet.myKey', 'myValue'); - }); - - it('handles complex values', async () => { - const complexValue = { nested: { data: [1, 2, 3] } }; - - const storage = makeNamespacedStorage('caplet', mockAdapter); - await storage.set('complex', complexValue); - - expect(mockAdapter.set).toHaveBeenCalledWith( - 'caplet.complex', - complexValue, - ); - }); - }); - - describe('delete', () => { - it('prefixes key with namespace', async () => { - const storage = makeNamespacedStorage('caplet', mockAdapter); - await storage.delete('myKey'); - - expect(mockAdapter.delete).toHaveBeenCalledWith('caplet.myKey'); - }); - }); - - describe('has', () => { - it('returns true when key exists', async () => { - vi.mocked(mockAdapter.get).mockResolvedValue('value'); - - const storage = makeNamespacedStorage('caplet', mockAdapter); - const result = await storage.has('myKey'); - - expect(result).toBe(true); - expect(mockAdapter.get).toHaveBeenCalledWith('caplet.myKey'); - }); - - it('returns false when key does not exist', async () => { - vi.mocked(mockAdapter.get).mockResolvedValue(undefined); - - const storage = makeNamespacedStorage('caplet', mockAdapter); - const result = await storage.has('nonExistent'); - - expect(result).toBe(false); - }); - }); - - describe('keys', () => { - it('returns keys with namespace prefix stripped', async () => { - vi.mocked(mockAdapter.keys).mockResolvedValue([ - 'caplet.key1', - 'caplet.key2', - 'caplet.nested.key', - ]); - - const storage = makeNamespacedStorage('caplet', mockAdapter); - const result = await storage.keys(); - - expect(result).toStrictEqual(['key1', 'key2', 'nested.key']); - expect(mockAdapter.keys).toHaveBeenCalledWith('caplet.'); - }); - - it('returns empty array when no keys in namespace', async () => { - vi.mocked(mockAdapter.keys).mockResolvedValue([]); - - const storage = makeNamespacedStorage('caplet', mockAdapter); - const result = await storage.keys(); - - expect(result).toStrictEqual([]); - }); - }); - - describe('clear', () => { - it('deletes all keys in namespace', async () => { - vi.mocked(mockAdapter.keys).mockResolvedValue([ - 'caplet.key1', - 'caplet.key2', - ]); - - const storage = makeNamespacedStorage('caplet', mockAdapter); - await storage.clear(); - - expect(mockAdapter.delete).toHaveBeenCalledTimes(2); - expect(mockAdapter.delete).toHaveBeenCalledWith('caplet.key1'); - expect(mockAdapter.delete).toHaveBeenCalledWith('caplet.key2'); - }); - - it('does nothing when namespace is empty', async () => { - vi.mocked(mockAdapter.keys).mockResolvedValue([]); - - const storage = makeNamespacedStorage('caplet', mockAdapter); - await storage.clear(); - - expect(mockAdapter.delete).not.toHaveBeenCalled(); - }); - }); - - describe('namespace isolation', () => { - it('uses different prefixes for different namespaces', async () => { - const storage1 = makeNamespacedStorage('caplet', mockAdapter); - const storage2 = makeNamespacedStorage('service', mockAdapter); - - await storage1.set('key', 'value1'); - await storage2.set('key', 'value2'); - - expect(mockAdapter.set).toHaveBeenCalledWith('caplet.key', 'value1'); - expect(mockAdapter.set).toHaveBeenCalledWith('service.key', 'value2'); - }); - }); -}); diff --git a/packages/omnium-gatherum/src/controllers/storage/namespaced-storage.ts b/packages/omnium-gatherum/src/controllers/storage/namespaced-storage.ts deleted file mode 100644 index 51e0c3eae..000000000 --- a/packages/omnium-gatherum/src/controllers/storage/namespaced-storage.ts +++ /dev/null @@ -1,52 +0,0 @@ -import type { Json } from '@metamask/utils'; - -import type { NamespacedStorage, StorageAdapter } from './types.ts'; - -/** - * Create a namespaced storage interface. - * All operations are scoped to the given namespace prefix. - * - * @param namespace - The namespace prefix for all keys. - * @param adapter - The underlying storage adapter. - * @returns A hardened NamespacedStorage instance. - */ -export function makeNamespacedStorage( - namespace: string, - adapter: StorageAdapter, -): NamespacedStorage { - const prefix = `${namespace}.`; - - const buildKey = (key: string): string => `${prefix}${key}`; - - const stripPrefix = (fullKey: string): string => fullKey.slice(prefix.length); - - return harden({ - async get(key: string): Promise { - return adapter.get(buildKey(key)); - }, - - async set(key: string, value: Json): Promise { - await adapter.set(buildKey(key), value); - }, - - async delete(key: string): Promise { - await adapter.delete(buildKey(key)); - }, - - async has(key: string): Promise { - const value = await adapter.get(buildKey(key)); - return value !== undefined; - }, - - async keys(): Promise { - const allKeys = await adapter.keys(prefix); - return allKeys.map(stripPrefix); - }, - - async clear(): Promise { - const allKeys = await this.keys(); - await Promise.all(allKeys.map(async (key) => this.delete(key))); - }, - }); -} -harden(makeNamespacedStorage); diff --git a/packages/omnium-gatherum/test/e2e/smoke.test.ts b/packages/omnium-gatherum/test/e2e/smoke.test.ts index 96640f725..f2ec0f92d 100644 --- a/packages/omnium-gatherum/test/e2e/smoke.test.ts +++ b/packages/omnium-gatherum/test/e2e/smoke.test.ts @@ -1,7 +1,7 @@ import { test, expect } from '@playwright/test'; import type { Page, BrowserContext } from '@playwright/test'; -import { loadExtension } from '../helpers.ts'; +import { loadExtension } from './utils.ts'; test.describe.configure({ mode: 'serial' }); diff --git a/packages/omnium-gatherum/test/helpers.ts b/packages/omnium-gatherum/test/e2e/utils.ts similarity index 96% rename from packages/omnium-gatherum/test/helpers.ts rename to packages/omnium-gatherum/test/e2e/utils.ts index a8306e37a..1caa88d6d 100644 --- a/packages/omnium-gatherum/test/helpers.ts +++ b/packages/omnium-gatherum/test/e2e/utils.ts @@ -6,7 +6,7 @@ export { sessionPath } from '@ocap/repo-tools/test-utils/extension'; const extensionPath = path.resolve( path.dirname(fileURLToPath(import.meta.url)), - '../dist', + '../../dist', ); export const loadExtension = async (contextId?: string) => { diff --git a/packages/omnium-gatherum/test/utils.ts b/packages/omnium-gatherum/test/utils.ts new file mode 100644 index 000000000..c6294a8ca --- /dev/null +++ b/packages/omnium-gatherum/test/utils.ts @@ -0,0 +1,31 @@ +import type { Json } from '@metamask/utils'; + +import type { StorageAdapter } from '../src/controllers/storage/types.ts'; + +/** + * Create a mock StorageAdapter for testing. + * + * @returns A mock storage adapter backed by an in-memory Map. + */ +export function makeMockStorageAdapter(): StorageAdapter { + const store = new Map(); + + return { + async get(key: string): Promise { + return store.get(key) as Value | undefined; + }, + async set(key: string, value: Json): Promise { + store.set(key, value); + }, + async delete(key: string): Promise { + store.delete(key); + }, + async keys(prefix?: string): Promise { + const allKeys = Array.from(store.keys()); + if (prefix === undefined) { + return allKeys; + } + return allKeys.filter((k) => k.startsWith(prefix)); + }, + }; +} From 588ac946c61ed30ae3f0845539bb2de63a04dd78 Mon Sep 17 00:00:00 2001 From: Erik Marks <25517051+rekmarks@users.noreply.github.com> Date: Fri, 9 Jan 2026 16:12:27 -0800 Subject: [PATCH 5/7] refactor(omnium): Simplify CapletId validation to allow any ASCII string Remove strict reverse DNS format requirement for CapletId to allow more flexibility during early development. Now accepts any non-empty ASCII string without whitespace, removing restrictions on hyphens, underscores, uppercase, and segment count. Co-Authored-By: Claude Sonnet 4.5 --- .../src/controllers/caplet/types.test.ts | 30 +++++++++---------- .../src/controllers/caplet/types.ts | 9 +++--- 2 files changed, 19 insertions(+), 20 deletions(-) diff --git a/packages/omnium-gatherum/src/controllers/caplet/types.test.ts b/packages/omnium-gatherum/src/controllers/caplet/types.test.ts index 2b1138f5f..a48da144a 100644 --- a/packages/omnium-gatherum/src/controllers/caplet/types.test.ts +++ b/packages/omnium-gatherum/src/controllers/caplet/types.test.ts @@ -10,25 +10,23 @@ import { describe('isCapletId', () => { it.each([ ['com.example.test', true], - ['org.metamask.keyring', true], - ['io.github.user.package', true], - ['a.b', true], - ['a1.b2', true], - ['test.caplet123', true], + ['simple', true], + ['bitcoin-signer', true], + ['test_caplet', true], + ['My-Caplet', true], + ['123', true], + ['a.b.c.d', true], ])('validates "%s" as %s', (value, expected) => { expect(isCapletId(value)).toBe(expected); }); it.each([ - ['', false], - ['single', false], // Must have at least 2 segments - ['com.Example.test', false], // No uppercase - ['com.123.test', false], // Segments cannot start with number - ['com..test', false], // Empty segment - ['com.test-name', false], // No hyphens - ['com.test_name', false], // No underscores - ['.com.test', false], // Cannot start with dot - ['com.test.', false], // Cannot end with dot + ['', false], // Empty + ['has space', false], // Whitespace + ['has\ttab', false], // Tab + ['has\nnewline', false], // Newline + ['café', false], // Non-ASCII + ['🎉', false], // Emoji [123, false], // Not a string [null, false], [undefined, false], @@ -93,7 +91,7 @@ describe('isCapletManifest', () => { }); it('rejects manifest with invalid id', () => { - expect(isCapletManifest({ ...validManifest, id: 'invalid' })).toBe(false); + expect(isCapletManifest({ ...validManifest, id: 'has space' })).toBe(false); }); it('rejects manifest with invalid version', () => { @@ -129,7 +127,7 @@ describe('assertCapletManifest', () => { }); it('throws for invalid manifest', () => { - expect(() => assertCapletManifest({ id: 'bad' })).toThrow( + expect(() => assertCapletManifest({ id: '' })).toThrow( 'Invalid CapletManifest', ); }); diff --git a/packages/omnium-gatherum/src/controllers/caplet/types.ts b/packages/omnium-gatherum/src/controllers/caplet/types.ts index cdf201be7..68a3ff10d 100644 --- a/packages/omnium-gatherum/src/controllers/caplet/types.ts +++ b/packages/omnium-gatherum/src/controllers/caplet/types.ts @@ -3,14 +3,13 @@ import type { Infer } from '@metamask/superstruct'; import semverValid from 'semver/functions/valid'; /** - * Unique identifier for a Caplet. - * Uses reverse domain notation (e.g., "com.example.bitcoin-signer"). + * Unique identifier for a Caplet (any non-empty ASCII string without whitespace). */ export type CapletId = string; /** * Validate CapletId format. - * Requires lowercase alphanumeric segments separated by dots, minimum 2 segments. + * Requires non-empty ASCII string with no whitespace. * * @param value - The value to validate. * @returns True if valid CapletId format. @@ -18,7 +17,9 @@ export type CapletId = string; export const isCapletId = (value: unknown): value is CapletId => typeof value === 'string' && value.length > 0 && - /^[a-z][a-z0-9]*(\.[a-z][a-z0-9]*)+$/u.test(value); + // eslint-disable-next-line no-control-regex + /^[\x00-\x7F]+$/u.test(value) && // ASCII only + !/\s/u.test(value); // No whitespace export const CapletIdStruct = define('CapletId', isCapletId); From ee93a066bfe4a4e65293503d6bb8f46de3da9030 Mon Sep 17 00:00:00 2001 From: Erik Marks <25517051+rekmarks@users.noreply.github.com> Date: Wed, 14 Jan 2026 22:01:19 -0800 Subject: [PATCH 6/7] chore: Update coverage --- vitest.config.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/vitest.config.ts b/vitest.config.ts index 07b25111d..35bac3850 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -75,10 +75,10 @@ export default defineConfig({ lines: 100, }, 'packages/extension/**': { - statements: 1.35, + statements: 1.29, functions: 0, branches: 0, - lines: 1.36, + lines: 1.31, }, 'packages/kernel-agents/**': { statements: 88.16, @@ -87,10 +87,10 @@ export default defineConfig({ lines: 88.13, }, 'packages/kernel-browser-runtime/**': { - statements: 84.4, + statements: 84.16, functions: 78.3, branches: 81.11, - lines: 84.63, + lines: 84.4, }, 'packages/kernel-errors/**': { statements: 99.24, @@ -165,10 +165,10 @@ export default defineConfig({ lines: 95.42, }, 'packages/omnium-gatherum/**': { - statements: 4.34, - functions: 4.76, - branches: 0, - lines: 4.41, + statements: 60.5, + functions: 61.44, + branches: 71.11, + lines: 60.42, }, 'packages/remote-iterables/**': { statements: 100, From d014ffd6ab6ffa6911eb72521800bd2536f6030a Mon Sep 17 00:00:00 2001 From: Erik Marks <25517051+rekmarks@users.noreply.github.com> Date: Thu, 15 Jan 2026 10:18:51 -0800 Subject: [PATCH 7/7] refactor: Fix omnium background globals --- packages/omnium-gatherum/src/background.ts | 98 ++++++++++++---------- 1 file changed, 55 insertions(+), 43 deletions(-) diff --git a/packages/omnium-gatherum/src/background.ts b/packages/omnium-gatherum/src/background.ts index 091e74dde..b73855fc6 100644 --- a/packages/omnium-gatherum/src/background.ts +++ b/packages/omnium-gatherum/src/background.ts @@ -19,19 +19,20 @@ import { CapletController, makeChromeStorageAdapter, } from './controllers/index.ts'; -import type { CapletManifest, LaunchResult } from './controllers/index.ts'; - -defineGlobals(); +import type { + CapletControllerFacet, + CapletManifest, + LaunchResult, +} from './controllers/index.ts'; const OFFSCREEN_DOCUMENT_PATH = '/offscreen.html'; const logger = new Logger('background'); +const globals = defineGlobals(); let bootPromise: Promise | null = null; -let kernelP: Promise; -let ping: () => Promise; // With this we can click the extension action button to wake up the service worker. chrome.action.onClicked.addListener(() => { - ping?.().catch(logger.error); + omnium.ping?.().catch(logger.error); }); // Install/update @@ -111,17 +112,13 @@ async function main(): Promise { }, }); - kernelP = backgroundCapTP.getKernel(); + const kernelP = backgroundCapTP.getKernel(); + globals.setKernelP(kernelP); - ping = async (): Promise => { + globals.setPing(async (): Promise => { const result = await E(kernelP).ping(); logger.info(result); - }; - - // Helper to get the kernel remote presence (for use with E()) - const getKernel = async (): Promise => { - return kernelP; - }; + }); // Create storage adapter const storageAdapter = makeChromeStorageAdapter(); @@ -162,28 +159,7 @@ async function main(): Promise { }, }, ); - - Object.defineProperties(globalThis.omnium, { - ping: { - value: ping, - }, - getKernel: { - value: getKernel, - }, - caplet: { - value: harden({ - install: async (manifest: CapletManifest, bundle?: unknown) => - E(capletController).install(manifest, bundle), - uninstall: async (capletId: string) => - E(capletController).uninstall(capletId), - list: async () => E(capletController).list(), - get: async (capletId: string) => E(capletController).get(capletId), - getByService: async (serviceName: string) => - E(capletController).getByService(serviceName), - }), - }, - }); - harden(globalThis.omnium); + globals.setCapletController(capletController); try { await offscreenStream.drain((message) => { @@ -203,10 +179,25 @@ async function main(): Promise { } } +type GlobalSetters = { + setKernelP: (value: Promise) => void; + setPing: (value: () => Promise) => void; + setCapletController: (value: CapletControllerFacet) => void; +}; + /** * Define globals accessible via the background console. + * + * @returns A device for setting the global values. */ -function defineGlobals(): void { +function defineGlobals(): GlobalSetters { + Object.defineProperty(globalThis, 'E', { + configurable: false, + enumerable: true, + writable: false, + value: E, + }); + Object.defineProperty(globalThis, 'omnium', { configurable: false, enumerable: true, @@ -214,6 +205,10 @@ function defineGlobals(): void { value: {}, }); + let kernelP: Promise; + let ping: (() => Promise) | undefined; + let capletController: CapletControllerFacet; + Object.defineProperties(globalThis.omnium, { ping: { get: () => ping, @@ -221,13 +216,30 @@ function defineGlobals(): void { getKernel: { value: async () => kernelP, }, + caplet: { + value: harden({ + install: async (manifest: CapletManifest, bundle?: unknown) => + E(capletController).install(manifest, bundle), + uninstall: async (capletId: string) => + E(capletController).uninstall(capletId), + list: async () => E(capletController).list(), + get: async (capletId: string) => E(capletController).get(capletId), + getByService: async (serviceName: string) => + E(capletController).getByService(serviceName), + }), + }, }); harden(globalThis.omnium); - Object.defineProperty(globalThis, 'E', { - configurable: false, - enumerable: true, - writable: false, - value: E, - }); + return { + setKernelP: (value) => { + kernelP = value; + }, + setPing: (value) => { + ping = value; + }, + setCapletController: (value) => { + capletController = value; + }, + }; }