fix: MetadataPlugin driver bridging and register() loader filtering

1. MetadataPlugin.start() now discovers driver services from kernel
   and calls manager.setDatabaseDriver() to enable DatabaseLoader.
2. register() only persists to 'datasource:' protocol loaders,
   preventing FilesystemLoader from being written to at runtime.
3. unregister() applies the same datasource: protocol filter.
4. Added 10 new tests covering all three fixes.

Agent-Logs-Url: https://github.com/objectstack-ai/framework/sessions/12f98d90-59fd-4398-acd1-e267ef7256cc

Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>

Author: Copilot

packages/metadata/src/metadata-manager.ts

@@ -153,17 +153,21 @@ export class MetadataManager implements IMetadataService {
 
   /**
    * Register/save a metadata item by type
-   * Stores in-memory registry and persists to writable loaders (if configured)
+   * Stores in-memory registry and persists to database-backed loaders only.
+   * FilesystemLoader (protocol 'file:') is read-only for static metadata and
+   * should not be written to during runtime registration.
    */
   async register(type: string, name: string, data: unknown): Promise<void> {
     if (!this.registry.has(type)) {
       this.registry.set(type, new Map());
     }
     this.registry.get(type)!.set(name, data);
 
-    // Persist to writable loaders (e.g., DatabaseLoader for history tracking)
+    // Persist only to database-backed loaders (protocol 'datasource:')
+    // FilesystemLoader is read-only at runtime — writing to it can crash in
+    // read-only environments (e.g. serverless, containerized deployments).
     for (const loader of this.loaders.values()) {
-      if (loader.save) {
+      if (loader.save && loader.contract.protocol === 'datasource:') {
         await loader.save(type, name, data);
       }
     }
@@ -218,7 +222,8 @@ export class MetadataManager implements IMetadataService {
   }
 
   /**
-   * Unregister/remove a metadata item by type and name
+   * Unregister/remove a metadata item by type and name.
+   * Deletes from database-backed loaders only (same rationale as register()).
    */
   async unregister(type: string, name: string): Promise<void> {
     // Remove from in-memory registry
@@ -230,9 +235,9 @@ export class MetadataManager implements IMetadataService {
       }
     }
 
-    // Also delete from all loaders that support deletion
+    // Delete only from database-backed loaders (protocol 'datasource:')
     for (const loader of this.loaders.values()) {
-      // Check if the loader has a delete method
+      if (loader.contract.protocol !== 'datasource:') continue;
       if (typeof (loader as any).delete === 'function') {
         try {
           await (loader as any).delete(type, name);

packages/metadata/src/metadata.test.ts

@@ -226,6 +226,118 @@ describe('MetadataManager', () => {
     });
   });
 
+  describe('register — loader protocol filtering', () => {
+    it('should persist to datasource: protocol loaders', async () => {
+      const dbLoader: MetadataLoader = {
+        contract: { name: 'database', protocol: 'datasource:' as const, capabilities: { read: true, write: true, watch: false, list: true } },
+        load: vi.fn().mockResolvedValue({ data: null }),
+        loadMany: vi.fn().mockResolvedValue([]),
+        exists: vi.fn().mockResolvedValue(false),
+        stat: vi.fn().mockResolvedValue(null),
+        list: vi.fn().mockResolvedValue([]),
+        save: vi.fn().mockResolvedValue({ success: true }),
+      };
+
+      const m = new MetadataManager({ formats: ['json'], loaders: [dbLoader] });
+      await m.register('object', 'account', { name: 'account' });
+
+      expect(dbLoader.save).toHaveBeenCalledWith('object', 'account', { name: 'account' });
+    });
+
+    it('should NOT persist to file: protocol loaders', async () => {
+      const fsLoader: MetadataLoader = {
+        contract: { name: 'filesystem', protocol: 'file:' as const, capabilities: { read: true, write: true, watch: true, list: true } },
+        load: vi.fn().mockResolvedValue({ data: null }),
+        loadMany: vi.fn().mockResolvedValue([]),
+        exists: vi.fn().mockResolvedValue(false),
+        stat: vi.fn().mockResolvedValue(null),
+        list: vi.fn().mockResolvedValue([]),
+        save: vi.fn().mockResolvedValue({ success: true }),
+      };
+
+      const m = new MetadataManager({ formats: ['json'], loaders: [fsLoader] });
+      await m.register('object', 'account', { name: 'account' });
+
+      expect(fsLoader.save).not.toHaveBeenCalled();
+    });
+
+    it('should NOT persist to memory: protocol loaders', async () => {
+      const memLoader: MetadataLoader = {
+        contract: { name: 'memory', protocol: 'memory:' as const, capabilities: { read: true, write: true, watch: false, list: true } },
+        load: vi.fn().mockResolvedValue({ data: null }),
+        loadMany: vi.fn().mockResolvedValue([]),
+        exists: vi.fn().mockResolvedValue(false),
+        stat: vi.fn().mockResolvedValue(null),
+        list: vi.fn().mockResolvedValue([]),
+        save: vi.fn().mockResolvedValue({ success: true }),
+      };
+
+      const m = new MetadataManager({ formats: ['json'], loaders: [memLoader] });
+      await m.register('object', 'account', { name: 'account' });
+
+      expect(memLoader.save).not.toHaveBeenCalled();
+    });
+
+    it('should still store in in-memory registry regardless of loaders', async () => {
+      const m = new MetadataManager({ formats: ['json'], loaders: [] });
+      await m.register('object', 'account', { name: 'account' });
+
+      const result = await m.get('object', 'account');
+      expect(result).toEqual({ name: 'account' });
+    });
+  });
+
+  describe('unregister — loader protocol filtering', () => {
+    it('should delete from datasource: protocol loaders', async () => {
+      const deleteFn = vi.fn();
+      const dbLoader: MetadataLoader = {
+        contract: { name: 'database', protocol: 'datasource:' as const, capabilities: { read: true, write: true, watch: false, list: true } },
+        load: vi.fn().mockResolvedValue({ data: null }),
+        loadMany: vi.fn().mockResolvedValue([]),
+        exists: vi.fn().mockResolvedValue(false),
+        stat: vi.fn().mockResolvedValue(null),
+        list: vi.fn().mockResolvedValue([]),
+        save: vi.fn().mockResolvedValue({ success: true }),
+        delete: deleteFn,
+      } as any;
+
+      const m = new MetadataManager({ formats: ['json'], loaders: [dbLoader] });
+      await m.register('object', 'account', { name: 'account' });
+      await m.unregister('object', 'account');
+
+      expect(deleteFn).toHaveBeenCalledWith('object', 'account');
+    });
+
+    it('should NOT delete from file: protocol loaders', async () => {
+      const deleteFn = vi.fn();
+      const fsLoader: MetadataLoader = {
+        contract: { name: 'filesystem', protocol: 'file:' as const, capabilities: { read: true, write: true, watch: true, list: true } },
+        load: vi.fn().mockResolvedValue({ data: null }),
+        loadMany: vi.fn().mockResolvedValue([]),
+        exists: vi.fn().mockResolvedValue(false),
+        stat: vi.fn().mockResolvedValue(null),
+        list: vi.fn().mockResolvedValue([]),
+        save: vi.fn().mockResolvedValue({ success: true }),
+        delete: deleteFn,
+      } as any;
+
+      const m = new MetadataManager({ formats: ['json'], loaders: [fsLoader] });
+      await m.register('object', 'account', { name: 'account' });
+      await m.unregister('object', 'account');
+
+      expect(deleteFn).not.toHaveBeenCalled();
+    });
+
+    it('should remove from in-memory registry', async () => {
+      const m = new MetadataManager({ formats: ['json'], loaders: [] });
+      await m.register('object', 'account', { name: 'account' });
+      expect(await m.get('object', 'account')).toEqual({ name: 'account' });
+
+      await m.unregister('object', 'account');
+      expect(await m.get('object', 'account')).toBeUndefined();
+    });
+  });
+
   describe('registerLoader', () => {
     it('should register a new loader', async () => {
       const newLoader = new MemoryLoader();
@@ -340,6 +452,7 @@ describe('MetadataPlugin', () => {
       registerLoader = vi.fn();
       stopWatching = vi.fn();
       setTypeRegistry = vi.fn();
+      setDatabaseDriver = vi.fn();
       register = vi.fn();
     };
     return { NodeMetadataManager: MockNodeMetadataManager };
@@ -382,6 +495,53 @@ describe('MetadataPlugin', () => {
     // start should call logger.info at least once
     expect(ctx.logger.info).toHaveBeenCalled();
   });
+
+  it('should bridge driver service to MetadataManager in start()', async () => {
+    const { MetadataPlugin } = await import('./plugin.js');
+    const plugin = new MetadataPlugin({ rootDir: '/tmp/test', watch: false });
+
+    const mockDriver = { name: 'mock-driver', find: vi.fn(), create: vi.fn() };
+    const services = new Map<string, any>();
+    services.set('driver.mock-driver', mockDriver);
+
+    const ctx = createMockPluginContext();
+    ctx.getServices = vi.fn().mockReturnValue(services);
+
+    await plugin.init(ctx);
+    await plugin.start(ctx);
+
+    // Verify setDatabaseDriver was called on the manager with the driver
+    const manager = (plugin as any).manager;
+    expect(manager.setDatabaseDriver).toHaveBeenCalledWith(mockDriver);
+  });
+
+  it('should not fail when no driver service is available', async () => {
+    const { MetadataPlugin } = await import('./plugin.js');
+    const plugin = new MetadataPlugin({ rootDir: '/tmp/test', watch: false });
+
+    const ctx = createMockPluginContext();
+    ctx.getServices = vi.fn().mockReturnValue(new Map());
+
+    await plugin.init(ctx);
+    // Should not throw
+    await expect(plugin.start(ctx)).resolves.not.toThrow();
+
+    // setDatabaseDriver should not have been called
+    const manager = (plugin as any).manager;
+    expect(manager.setDatabaseDriver).not.toHaveBeenCalled();
+  });
+
+  it('should gracefully handle getServices errors', async () => {
+    const { MetadataPlugin } = await import('./plugin.js');
+    const plugin = new MetadataPlugin({ rootDir: '/tmp/test', watch: false });
+
+    const ctx = createMockPluginContext();
+    ctx.getServices = vi.fn().mockImplementation(() => { throw new Error('services unavailable'); });
+
+    await plugin.init(ctx);
+    // Should not throw even when getServices fails
+    await expect(plugin.start(ctx)).resolves.not.toThrow();
+  });
 });
 
 // ---------- Helpers ----------

packages/metadata/src/plugin.ts

@@ -69,6 +69,26 @@ export class MetadataPlugin implements Plugin {
     }
 
     start = async (ctx: PluginContext) => {
+        // Bridge database driver from kernel service registry to MetadataManager.
+        // DriverPlugin registers drivers as 'driver.{name}' services during init().
+        // We look them up here (start phase) to enable DatabaseLoader persistence.
+        try {
+            const services = ctx.getServices();
+            for (const [serviceName, service] of services) {
+                if (serviceName.startsWith('driver.') && service) {
+                    ctx.logger.info('[MetadataPlugin] Bridging driver to MetadataManager for database-backed persistence', {
+                        driverService: serviceName,
+                    });
+                    this.manager.setDatabaseDriver(service);
+                    break; // Use the first available driver
+                }
+            }
+        } catch (e: any) {
+            ctx.logger.debug('[MetadataPlugin] No driver service found — database metadata persistence not available', {
+                error: e.message,
+            });
+        }
+
         ctx.logger.info('Loading metadata from file system...');
 
         // Use the type registry to discover metadata types (sorted by loadOrder)