Plugins are switched on and off in a Plugins tab, the server keeps the state in data/plugins/plugins.json, and switched-off plugins are never delivered (#114)

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

tests/editor/plugins.test.ts

@@ -1,7 +1,8 @@
 /**
- * Plugins (#113): what the loader does with every way a file can fail to be a plugin, and what the
- * API v1 lets a plugin see and do. The node board itself draws text and is checked in the browser
- * (e2e step 38); only its reading of plugin input is here.
+ * Plugins (#113, #114): what the loader does with every way a file can fail to be a plugin, what
+ * the API v1 lets a plugin see and do, and what switching one on and off in the Plugins tab does.
+ * The node board itself draws text and is checked in the browser (e2e step 38); only its reading
+ * of plugin input is here.
  */
 import { afterEach, describe, expect, it, vi } from 'vitest';
 import {
@@ -11,9 +12,10 @@ import {
 } from '../../src/core/commands/assetCommands';
 import { createEmptyProject } from '../../src/core/model/Project';
 import { zoomedAbout } from '../../src/editor/graph/graphView';
-import { activatePlugins, readMeta } from '../../src/editor/plugins/loader';
+import { PluginManager, type PluginRecord, PluginScope, readMeta } from '../../src/editor/plugins/loader';
 import { boardNodes } from '../../src/editor/plugins/NodeBoard';
-import { createPluginApi, type PluginHost } from '../../src/editor/plugins/pluginApi';
+import { displayName, statusText } from '../../src/editor/plugins/PluginsPane';
+import { createPluginApi, type PluginHost, scopedHost } from '../../src/editor/plugins/pluginApi';
 import { EditorState } from '../../src/editor/state';
 import { Widget } from '../../src/ui';
 
@@ -22,36 +24,50 @@ const plugin = (id: string, activate: (api: unknown) => unknown = () => {}, api:
   activate,
 });
 
-function loaderDeps(modules: Record<string, () => unknown>) {
+/**
+ * A manager on made-up modules. Every file is switched on unless `off` names it, which is what
+ * the server's `plugins.json` says in the real thing.
+ */
+function loaderDeps(modules: Record<string, () => unknown>, off: string[] = []) {
   const reports: string[] = [];
   const activated: string[] = [];
-  return {
-    reports,
-    activated,
-    deps: {
-      load: (file: string) => {
-        const make = modules[file];
-        return make ? Promise.resolve(make()) : Promise.reject(new Error('404'));
-      },
-      api: (meta: { id: string }) => {
-        activated.push(meta.id);
-        return { for: meta.id };
-      },
-      report: (message: string) => reports.push(message),
+  const stored: [string, boolean][] = [];
+  const loads: [string, number][] = [];
+  const deps = {
+    load: (file: string, updatedAt: number) => {
+      loads.push([file, updatedAt]);
+      const make = modules[file];
+      return make ? Promise.resolve(make()) : Promise.reject(new Error('404'));
     },
+    api: (meta: { id: string }) => {
+      activated.push(meta.id);
+      return { for: meta.id };
+    },
+    store: (file: string, active: boolean) => {
+      stored.push([file, active]);
+      return Promise.resolve();
+    },
+    report: (message: string) => reports.push(message),
   };
+  const manager = new PluginManager(deps);
+  const start = (files = Object.keys(modules)) =>
+    manager.start(files.map((file, i) => ({ file, updatedAt: i + 1, active: !off.includes(file) })));
+  return { reports, activated, stored, loads, deps, manager, start };
 }
 
+const byFileOf = (records: readonly PluginRecord[]) =>
+  Object.fromEntries(records.map((r) => [r.file, r])) as Record<string, PluginRecord | undefined>;
+
 describe('the plugin loader', () => {
   it('activates in file-name order, whatever order the list came in', async () => {
     const order: string[] = [];
-    const { deps } = loaderDeps({
+    const { manager, start } = loaderDeps({
       'b.js': () => plugin('b', () => order.push('b')),
       'a.js': () => plugin('a', () => order.push('a')),
     });
-    const records = await activatePlugins(['b.js', 'a.js'], deps);
+    await start(['b.js', 'a.js']);
     expect(order).toEqual(['a', 'b']);
-    expect(records.map((r) => [r.file, r.status])).toEqual([
+    expect(manager.records.map((r) => [r.file, r.status])).toEqual([
       ['a.js', 'active'],
       ['b.js', 'active'],
     ]);
@@ -60,7 +76,7 @@ describe('the plugin loader', () => {
   it('hands every plugin its own API and waits for an async activate before the next', async () => {
     const seen: unknown[] = [];
     let first = false;
-    const { deps } = loaderDeps({
+    const { start } = loaderDeps({
       'a.js': () =>
         plugin('a', async (api) => {
           seen.push(api);
@@ -69,12 +85,12 @@ describe('the plugin loader', () => {
         }),
       'b.js': () => plugin('b', (api) => seen.push(api, first)),
     });
-    await activatePlugins(['a.js', 'b.js'], deps);
+    await start();
     expect(seen).toEqual([{ for: 'a' }, { for: 'b' }, true]);
   });
 
   it('reports each broken plugin and goes on with the rest', async () => {
-    const { deps, reports, activated } = loaderDeps({
+    const { manager, start, reports, activated } = loaderDeps({
       'good.js': () => plugin('good'),
       'no-meta.js': () => ({ activate: () => {} }),
       'no-activate.js': () => ({ meta: { id: 'x', api: 1 } }),
@@ -84,11 +100,8 @@ describe('the plugin loader', () => {
           throw new Error('boom');
         }),
     });
-    const records = await activatePlugins(
-      ['missing.js', 'no-meta.js', 'no-activate.js', 'future.js', 'throws.js', 'good.js'],
-      deps,
-    );
-    const byFile = Object.fromEntries(records.map((r) => [r.file, r]));
+    await start(['missing.js', 'no-meta.js', 'no-activate.js', 'future.js', 'throws.js', 'good.js']);
+    const byFile = byFileOf(manager.records);
     expect(byFile['good.js']?.status).toBe('active');
     expect(byFile['missing.js']).toMatchObject({ status: 'failed', error: 'does not load (404)' });
     expect(byFile['no-meta.js']).toMatchObject({ status: 'failed', error: 'exports no meta' });
@@ -108,25 +121,26 @@ describe('the plugin loader', () => {
   });
 
   it('turns away a second plugin with an id that is active already', async () => {
-    const { deps, activated } = loaderDeps({ 'a.js': () => plugin('same'), 'b.js': () => plugin('same') });
-    const records = await activatePlugins(['a.js', 'b.js'], deps);
-    expect(records.map((r) => r.status)).toEqual(['active', 'failed']);
-    expect(records[1]?.error).toBe('a plugin with the id "same" is active already');
+    const { manager, start, activated } = loaderDeps({
+      'a.js': () => plugin('same'),
+      'b.js': () => plugin('same'),
+    });
+    await start();
+    expect(manager.records.map((r) => r.status)).toEqual(['active', 'failed']);
+    expect(manager.records[1]?.error).toBe('a plugin with the id "same" is active already');
     expect(activated).toEqual(['same']);
   });
 
   it('lets a failed plugin keep its id free for a later one', async () => {
-    const { deps } = loaderDeps({
+    const { manager, start } = loaderDeps({
       'a.js': () =>
         plugin('same', () => {
           throw new Error('no');
         }),
       'b.js': () => plugin('same'),
     });
-    expect((await activatePlugins(['a.js', 'b.js'], deps)).map((r) => r.status)).toEqual([
-      'failed',
-      'active',
-    ]);
+    await start();
+    expect(manager.records.map((r) => r.status)).toEqual(['failed', 'active']);
   });
 
   it('fills in a missing name and version from what is there', () => {
@@ -136,6 +150,9 @@ describe('the plugin loader', () => {
       version: '',
       api: 1,
     });
+    expect(readMeta({ meta: { id: 'x', api: 1 }, activate: () => {}, deactivate: 'no' })).toBe(
+      'deactivate must be a function',
+    );
     expect(readMeta(null)).toBe('exports no meta');
     expect(readMeta({ meta: { id: '', api: 1 }, activate: () => {} })).toBe(
       'meta.id must be a non-empty string',
@@ -158,6 +175,7 @@ function editorHost(): {
     onProjectChange: (listener) => state.on('project', listener),
     hasTab: (id) => id === 'scene' || tabs.has(id),
     addTab: (id, label, content) => tabs.set(id, { label, content }),
+    removeTab: (id) => void tabs.delete(id),
   };
   return { state, host, tabs };
 }
@@ -258,6 +276,223 @@ describe('the plugin API', () => {
   });
 });
 
+/**
+ * Switching plugins on and off (#114). The switch is the server's, so it is stored before it is
+ * applied; applying it means loading and activating, or running `deactivate` and taking back
+ * everything the plugin registered through its API.
+ */
+describe('switching a plugin on and off', () => {
+  /** A manager over a real API and host, so what a plugin registers is really taken away again. */
+  function switchable(modules: Record<string, () => unknown>, off: string[] = []) {
+    const { state, host, tabs } = editorHost();
+    const reports: string[] = [];
+    const stored: [string, boolean][] = [];
+    const loads: [string, number][] = [];
+    let refuse = false;
+    const manager = new PluginManager({
+      load: (file, updatedAt) => {
+        loads.push([file, updatedAt]);
+        const make = modules[file];
+        return make ? Promise.resolve(make()) : Promise.reject(new Error('404'));
+      },
+      api: (meta, scope) => createPluginApi(meta.id, scopedHost(host, scope)),
+      store: (file, active) => {
+        if (refuse) return Promise.reject(new Error('Server error'));
+        stored.push([file, active]);
+        return Promise.resolve();
+      },
+      report: (message) => reports.push(message),
+    });
+    const start = () =>
+      manager.start(
+        Object.keys(modules).map((file, i) => ({ file, updatedAt: i + 1, active: !off.includes(file) })),
+      );
+    return { state, tabs, manager, start, reports, stored, loads, refuse: (no: boolean) => (refuse = no) };
+  }
+
+  /** A plugin that puts a tab and a change listener in place, and says when it was let go. */
+  const tabPlugin = (id: string, log: string[] = []) => ({
+    meta: { id, name: `Plugin ${id}`, version: '2.0', api: 1 },
+    activate: (api: ReturnType<typeof createPluginApi>) => {
+      api.project.on('change', () => log.push(`${id}: change`));
+      api.tabs.add({ id, label: id, create: () => new Pane() });
+    },
+    deactivate: () => log.push(`${id}: deactivate`),
+  });
+
+  it('loads only what is switched on – a plugin that is off is never even fetched', async () => {
+    const { manager, start, loads } = switchable(
+      { 'on.js': () => plugin('on'), 'off.js': () => plugin('off') },
+      ['off.js'],
+    );
+    await start();
+    expect(loads.map(([file]) => file)).toEqual(['on.js']);
+    expect(manager.records.map((r) => [r.file, r.status, r.meta?.name])).toEqual([
+      ['off.js', 'inactive', undefined],
+      ['on.js', 'active', 'Plugin on'],
+    ]);
+  });
+
+  it('stores the switch first, then loads and activates the plugin', async () => {
+    const log: string[] = [];
+    const { manager, start, stored, loads, tabs } = switchable({ 'demo.js': () => tabPlugin('demo', log) }, [
+      'demo.js',
+    ]);
+    await start();
+    expect(tabs.size).toBe(0);
+
+    await manager.setActive('demo.js', true);
+    expect(stored).toEqual([['demo.js', true]]);
+    // The file's own time goes into the load, so a replaced file is not taken from the module cache.
+    expect(loads).toEqual([['demo.js', 1]]);
+    expect(tabs.has('demo')).toBe(true);
+    expect(manager.records[0]).toMatchObject({ status: 'active', meta: { name: 'Plugin demo' } });
+  });
+
+  it('switching off runs deactivate and takes the tab and the listeners with it', async () => {
+    const log: string[] = [];
+    const { manager, start, state, tabs, stored } = switchable({ 'demo.js': () => tabPlugin('demo', log) });
+    await start();
+    state.execute(new CreateAssetCommand('sprite', 'Crate'));
+    expect(log).toEqual(['demo: change']);
+
+    await manager.setActive('demo.js', false);
+    expect(log).toEqual(['demo: change', 'demo: deactivate']);
+    expect(tabs.size).toBe(0);
+    expect(stored).toEqual([['demo.js', false]]);
+    expect(manager.records[0]).toMatchObject({ status: 'inactive' });
+    // Nothing of it runs any more, while the name it had stays on the row.
+    state.execute(new CreateAssetCommand('sprite', 'Box'));
+    expect(log).toEqual(['demo: change', 'demo: deactivate']);
+    expect(manager.records[0]?.meta?.name).toBe('Plugin demo');
+
+    await manager.setActive('demo.js', true);
+    state.execute(new CreateAssetCommand('sprite', 'Barrel'));
+    expect(log.at(-1)).toBe('demo: change');
+    expect(tabs.has('demo')).toBe(true);
+  });
+
+  it('leaves the plugin as it was when the server does not store the switch', async () => {
+    const { manager, start, reports, tabs, refuse } = switchable({ 'demo.js': () => tabPlugin('demo') });
+    await start();
+    refuse(true);
+    await manager.setActive('demo.js', false);
+    expect(manager.records[0]?.status).toBe('active');
+    expect(tabs.has('demo')).toBe(true);
+    expect(reports).toEqual(['Plugin demo.js not switched off: Server error']);
+  });
+
+  it('takes back what a plugin registered before its activate threw', async () => {
+    const { manager, start, tabs, reports } = switchable({
+      'half.js': () => ({
+        meta: { id: 'half', name: 'Half', version: '1', api: 1 },
+        activate: (api: ReturnType<typeof createPluginApi>) => {
+          api.tabs.add({ id: 'half', label: 'Half', create: () => new Pane() });
+          throw new Error('boom');
+        },
+      }),
+    });
+    await start();
+    expect(manager.records[0]).toMatchObject({ status: 'failed', error: 'activate failed (boom)' });
+    expect(tabs.size).toBe(0);
+    expect(reports).toEqual(['Plugin half.js not activated: activate failed (boom)']);
+    // Switched off it is simply off: nothing left to tidy, nothing to report, and the name it
+    // managed to give stays on its row.
+    await manager.setActive('half.js', false);
+    expect(manager.records[0]).toEqual({
+      file: 'half.js',
+      status: 'inactive',
+      meta: { id: 'half', name: 'Half', version: '1', api: 1 },
+    });
+  });
+
+  it('switches off even when the plugin throws on the way out, and says so', async () => {
+    const { manager, start, tabs, reports } = switchable({
+      'rude.js': () => ({
+        meta: { id: 'rude', name: 'Rude', version: '1', api: 1 },
+        activate: (api: ReturnType<typeof createPluginApi>) =>
+          api.tabs.add({ id: 'rude', label: 'Rude', create: () => new Pane() }),
+        deactivate: () => {
+          throw new Error('not going');
+        },
+      }),
+    });
+    await start();
+    await manager.setActive('rude.js', false);
+    expect(manager.records[0]).toMatchObject({
+      status: 'inactive',
+      error: 'deactivate failed (not going)',
+    });
+    expect(tabs.size).toBe(0);
+    expect(reports).toEqual(['Plugin rude.js switched off, but: deactivate failed (not going)']);
+  });
+
+  it('runs switches one at a time, in the order they were asked for', async () => {
+    const log: string[] = [];
+    const { manager, start, stored } = switchable({ 'demo.js': () => tabPlugin('demo', log) });
+    await start();
+    await Promise.all([
+      manager.setActive('demo.js', false),
+      manager.setActive('demo.js', true),
+      manager.setActive('demo.js', false),
+    ]);
+    expect(stored).toEqual([
+      ['demo.js', false],
+      ['demo.js', true],
+      ['demo.js', false],
+    ]);
+    expect(manager.records[0]?.status).toBe('inactive');
+  });
+
+  it('tells its listeners after every change', async () => {
+    const { manager, start } = switchable({ 'demo.js': () => tabPlugin('demo') });
+    const seen = vi.fn();
+    const off = manager.onChange(seen);
+    await start();
+    expect(seen).toHaveBeenCalledTimes(1);
+    await manager.setActive('demo.js', false);
+    expect(seen).toHaveBeenCalledTimes(2);
+    off();
+    await manager.setActive('demo.js', true);
+    expect(seen).toHaveBeenCalledTimes(2);
+  });
+
+  it('runs a scope’s cleanups newest first, all of them even when one throws', () => {
+    const scope = new PluginScope();
+    const order: string[] = [];
+    scope.add(() => order.push('first'));
+    scope.add(() => {
+      throw new Error('middle broke');
+    });
+    scope.add(() => order.push('last'));
+    expect(scope.dispose()).toEqual(['middle broke']);
+    expect(order).toEqual(['last', 'first']);
+    // Disposed twice (switched off, then the editor tidies up) runs nothing a second time.
+    expect(scope.dispose()).toEqual([]);
+  });
+
+  it('says on the row what became of each plugin', () => {
+    const row = (record: Partial<PluginRecord>): PluginRecord => ({
+      file: 'x.js',
+      status: 'inactive',
+      meta: null,
+      ...record,
+    });
+    expect(statusText(row({ status: 'active' }))).toBe('Active');
+    expect(statusText(row({}))).toBe('Off');
+    expect(statusText(row({ status: 'failed', error: 'does not load (404)' }))).toBe(
+      'Failed: does not load (404)',
+    );
+    expect(statusText(row({ error: 'deactivate failed (x)' }))).toBe(
+      'Switched off, but: deactivate failed (x)',
+    );
+    expect(displayName(row({}))).toBe('—');
+    expect(displayName(row({ meta: { id: 'demo', name: 'Demo plugin', version: '1', api: 1 } }))).toBe(
+      'Demo plugin',
+    );
+  });
+});
+
 describe('a node board reading plugin input', () => {
   it('takes titles and positions as given', () => {
     expect(boardNodes([{ id: 'a', title: 'Crate', x: 10, y: -20 }])).toEqual([