mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-18 21:16:47 +00:00
Feat/plugin instance actions (#2209)
* feat(shared): scope plugin action descriptors * feat(plugin-sdk): instance-scoped settings actions * feat(plugins): run instance-scoped actions from the admin settings dialog * feat(admin): instance-scoped plugin actions in the settings dialog * fix(plugins): surface instance actions without instance fields and harden the action gate * docs(plugin-sdk): note that hosts before 4.2.0 ignore action scope * fix(admin): open the dialog for instance actions, explain inactive 404s, guard the save window * docs(wiki): plugin pages for instance actions and the admin panel changes
This commit is contained in:
jubnl
@@ -30,7 +30,7 @@ beforeEach(() => {
|
||||
api_version INTEGER, min_trek_version TEXT, trek_range TEXT, permissions TEXT, capabilities TEXT DEFAULT '{}', dependencies TEXT DEFAULT '{}', operator_egress INTEGER DEFAULT 0, granted_permissions TEXT, status TEXT, config TEXT, updated_at TEXT);
|
||||
CREATE TABLE plugin_settings_fields (plugin_id TEXT, field_key TEXT, label TEXT, input_type TEXT, placeholder TEXT, hint TEXT,
|
||||
required INTEGER, secret INTEGER, scope TEXT, options TEXT, oauth_config TEXT, default_value TEXT, sort_order INTEGER);
|
||||
CREATE TABLE plugin_actions (plugin_id TEXT, action_key TEXT, label TEXT, hint TEXT, danger INTEGER, sort_order INTEGER,
|
||||
CREATE TABLE plugin_actions (plugin_id TEXT, action_key TEXT, label TEXT, hint TEXT, danger INTEGER, scope TEXT NOT NULL DEFAULT 'user', sort_order INTEGER,
|
||||
PRIMARY KEY (plugin_id, action_key));
|
||||
CREATE TABLE plugin_error_log (id INTEGER PRIMARY KEY AUTOINCREMENT, plugin_id TEXT, level TEXT, message TEXT, ts TEXT);`);
|
||||
});
|
||||
@@ -64,6 +64,19 @@ describe('discoverPlugins', () => {
|
||||
expect(field).toMatchObject({ field_key: 'api_key', secret: 1 });
|
||||
});
|
||||
|
||||
it('persists each action with its scope', () => {
|
||||
writePlugin('acts', {
|
||||
name: 'Acts', type: 'integration', permissions: [],
|
||||
actions: [{ key: 'ping', label: 'Ping' }, { key: 'purge', label: 'Purge', scope: 'instance', danger: true }],
|
||||
});
|
||||
discoverPlugins(db);
|
||||
const rows = db.prepare("SELECT action_key, scope, danger FROM plugin_actions WHERE plugin_id='acts' ORDER BY sort_order").all();
|
||||
expect(rows).toEqual([
|
||||
{ action_key: 'ping', scope: 'user', danger: 0 },
|
||||
{ action_key: 'purge', scope: 'instance', danger: 1 },
|
||||
]);
|
||||
});
|
||||
|
||||
it('keeps an existing plugin status + granted permissions on re-discovery', () => {
|
||||
db.prepare("INSERT INTO plugins (id, name, type, status, granted_permissions) VALUES ('keep','Keep','page','active','[\"db:own\"]')").run();
|
||||
writePlugin('keep', { name: 'Keep v2', type: 'page', version: '2.0.0' });
|
||||
|
||||
@@ -330,11 +330,21 @@ describe('settings-page actions', () => {
|
||||
it('parses actions with label/hint/danger', () => {
|
||||
const m = parseManifest({ ...base, actions: [{ key: 'testConnection', label: 'Test connection', hint: 'Pings the API.' }, { key: 'purge', label: 'Purge', danger: true }] });
|
||||
expect(m.actions).toEqual([
|
||||
{ key: 'testConnection', label: 'Test connection', hint: 'Pings the API.', danger: false },
|
||||
{ key: 'purge', label: 'Purge', hint: undefined, danger: true },
|
||||
{ key: 'testConnection', label: 'Test connection', hint: 'Pings the API.', danger: false, scope: 'user' },
|
||||
{ key: 'purge', label: 'Purge', hint: undefined, danger: true, scope: 'user' },
|
||||
]);
|
||||
});
|
||||
|
||||
it('defaults an action to the user scope, accepts instance, refuses anything else', () => {
|
||||
const m = parseManifest({ ...base, actions: [{ key: 'ping' }, { key: 'purge', scope: 'instance' }, { key: 'me', scope: 'user' }] });
|
||||
expect(m.actions.map((a) => a.scope)).toEqual(['user', 'instance', 'user']);
|
||||
expect(() => parseManifest({ ...base, actions: [{ key: 'x', scope: 'global' }] })).toThrow(/scope must be "user" or "instance"/);
|
||||
// one key, one form — a duplicate across scopes is still a duplicate
|
||||
expect(() => parseManifest({ ...base, actions: [{ key: 'a', scope: 'user' }, { key: 'a', scope: 'instance' }] })).toThrow(/duplicate action/);
|
||||
// the cap counts both scopes
|
||||
expect(() => parseManifest({ ...base, actions: Array.from({ length: 9 }, (_, i) => ({ key: `a${i}`, scope: i % 2 ? 'instance' : 'user' })) })).toThrow(/at most 8/);
|
||||
});
|
||||
|
||||
it('defaults the label to the key, and bounds label/hint', () => {
|
||||
const m = parseManifest({ ...base, actions: [{ key: 'sync', label: 'L'.repeat(200), hint: 'H'.repeat(500) }] });
|
||||
expect(m.actions[0].label.length).toBe(60);
|
||||
@@ -342,6 +352,11 @@ describe('settings-page actions', () => {
|
||||
expect(parseManifest({ ...base, actions: [{ key: 'sync' }] }).actions[0].label).toBe('sync');
|
||||
});
|
||||
|
||||
it('falls back to the key when label is an empty (or whitespace-only) string, not just null/undefined', () => {
|
||||
expect(parseManifest({ ...base, actions: [{ key: 'sync', label: '' }] }).actions[0].label).toBe('sync');
|
||||
expect(parseManifest({ ...base, actions: [{ key: 'sync', label: ' ' }] }).actions[0].label).toBe('sync');
|
||||
});
|
||||
|
||||
it('rejects a prototype-chain key, a duplicate, a non-array and too many', () => {
|
||||
expect(() => parseManifest({ ...base, actions: [{ key: '__proto__' }] })).toThrow(ManifestError);
|
||||
expect(() => parseManifest({ ...base, actions: [{ key: 'a' }, { key: 'a' }] })).toThrow(/duplicate action/);
|
||||
|
||||
@@ -127,15 +127,17 @@ describe('operator-supplied egress hosts', () => {
|
||||
});
|
||||
|
||||
describe('settings-page actions (runtime)', () => {
|
||||
function declareAction(id: string, key: string) {
|
||||
testDb.prepare('INSERT OR REPLACE INTO plugin_actions (plugin_id, action_key, label, hint, danger, sort_order) VALUES (?, ?, ?, NULL, 0, 0)')
|
||||
.run(id, key, key);
|
||||
function declareAction(id: string, key: string, scope: 'user' | 'instance' = 'user') {
|
||||
testDb.prepare('INSERT OR REPLACE INTO plugin_actions (plugin_id, action_key, label, hint, danger, scope, sort_order) VALUES (?, ?, ?, NULL, 0, ?, 0)')
|
||||
.run(id, key, key, scope);
|
||||
}
|
||||
|
||||
it('ACT-001 — actionsOf returns the declared descriptors', () => {
|
||||
it('ACT-001 — actionsOf returns the declared descriptors of ONE scope', () => {
|
||||
install('p', false);
|
||||
declareAction('p', 'testConnection');
|
||||
expect(rt.actionsOf('p')).toEqual([{ key: 'testConnection', label: 'testConnection', hint: undefined, danger: false }]);
|
||||
declareAction('p', 'purge', 'instance');
|
||||
expect(rt.actionsOf('p', 'user')).toEqual([{ key: 'testConnection', label: 'testConnection', hint: undefined, danger: false, scope: 'user' }]);
|
||||
expect(rt.actionsOf('p', 'instance')).toEqual([{ key: 'purge', label: 'purge', hint: undefined, danger: false, scope: 'instance' }]);
|
||||
});
|
||||
|
||||
it('ACT-002 — invoking an action the plugin never declared is REFUSED', async () => {
|
||||
@@ -143,13 +145,21 @@ describe('settings-page actions (runtime)', () => {
|
||||
declareAction('p', 'testConnection');
|
||||
// The key is caller-supplied (it comes off the URL), so the host must check it
|
||||
// against the manifest rather than forwarding whatever it is handed to the child.
|
||||
await expect(rt.invokeAction('p', 'somethingElse', 1)).rejects.toThrow(/did not declare action/);
|
||||
await expect(rt.invokeAction('p', '__proto__', 1)).rejects.toThrow(/did not declare action/);
|
||||
await expect(rt.invokeAction('p', 'somethingElse', 1, 'user')).rejects.toThrow(/did not declare action/);
|
||||
await expect(rt.invokeAction('p', '__proto__', 1, 'user')).rejects.toThrow(/did not declare action/);
|
||||
});
|
||||
|
||||
it('ACT-003 — a plugin with no actions can never be invoked', async () => {
|
||||
install('p', false);
|
||||
await expect(rt.invokeAction('p', 'testConnection', 1)).rejects.toThrow(/did not declare action/);
|
||||
await expect(rt.invokeAction('p', 'testConnection', 1, 'user')).rejects.toThrow(/did not declare action/);
|
||||
});
|
||||
|
||||
it('ACT-004 — a key declared in the OTHER scope is refused (the user route cannot fire an admin button)', async () => {
|
||||
install('p', false);
|
||||
declareAction('p', 'purge', 'instance');
|
||||
declareAction('p', 'testConnection', 'user');
|
||||
await expect(rt.invokeAction('p', 'purge', 1, 'user')).rejects.toThrow(/did not declare action "purge" in scope user/);
|
||||
await expect(rt.invokeAction('p', 'testConnection', 1, 'instance')).rejects.toThrow(/did not declare action "testConnection" in scope instance/);
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -11,6 +11,7 @@ import { HttpException } from '@nestjs/common';
|
||||
import fs from 'node:fs';
|
||||
import os from 'node:os';
|
||||
import path from 'node:path';
|
||||
import type { Request } from 'express';
|
||||
|
||||
const { testDb, dbMock } = vi.hoisted(() => {
|
||||
const Database = require('better-sqlite3');
|
||||
@@ -120,6 +121,22 @@ describe('instance settings fields', () => {
|
||||
expect(plugins.find((p) => p.id === 'plain')).toMatchObject({ instanceSettingsCount: 0 });
|
||||
});
|
||||
|
||||
it('INS-004 — the admin list carries the instance-action count (gates the menu item even with no settings fields)', () => {
|
||||
install('with-action');
|
||||
install('plain');
|
||||
testDb
|
||||
.prepare('INSERT INTO plugin_actions (plugin_id, action_key, label, hint, danger, scope, sort_order) VALUES (?, ?, ?, NULL, 0, ?, 0)')
|
||||
.run('with-action', 'purge', 'Purge', 'instance');
|
||||
testDb
|
||||
.prepare('INSERT INTO plugin_actions (plugin_id, action_key, label, hint, danger, scope, sort_order) VALUES (?, ?, ?, NULL, 0, ?, 0)')
|
||||
.run('with-action', 'notify', 'Notify', 'user'); // user-scope actions must not count
|
||||
|
||||
const plugins = svc().list().plugins;
|
||||
expect(plugins.find((p) => p.id === 'with-action')).toMatchObject({ instanceSettingsCount: 0, instanceActionsCount: 1 });
|
||||
expect(plugins.find((p) => p.id === 'plain')).toMatchObject({ instanceSettingsCount: 0, instanceActionsCount: 0 });
|
||||
testDb.prepare("DELETE FROM plugin_actions WHERE plugin_id = 'with-action'").run();
|
||||
});
|
||||
|
||||
it('INS-010 — persists a settings-field default and serves it on the fields list', () => {
|
||||
installFixturePlugin({ settings: [{ key: 'oauth_authorize_url', required: true, default: 'https://auth.openbnb.org/authorize' }] });
|
||||
const fields = svc().instanceSettingsFields('fixture-id');
|
||||
@@ -227,7 +244,7 @@ describe('admin config endpoints (controller)', () => {
|
||||
declareField('p', 'apiUrl', 'instance');
|
||||
testDb.prepare("UPDATE plugins SET config = '{\"apiUrl\":\"https://x.example\"}' WHERE id = 'p'").run();
|
||||
|
||||
const out = controllerWith({}).getConfig('p');
|
||||
const out = controllerWith({ actionsOf: () => [] }).getConfig('p');
|
||||
expect(out.config).toEqual({ apiUrl: 'https://x.example' });
|
||||
expect(out.fields.map((f: Record<string, unknown>) => f.key)).toEqual(['apiUrl']);
|
||||
});
|
||||
@@ -328,3 +345,63 @@ describe('defaults reach the child at spawn', () => {
|
||||
expect(config).toEqual({ api_url: 'https://mine.example', retries: 3 }); // note: no default, no value → absent
|
||||
});
|
||||
});
|
||||
|
||||
describe('plugin_actions.scope migration', () => {
|
||||
it('MIG-ACT-001 — the column exists on a migrated DB and defaults to user', () => {
|
||||
const cols = testDb.prepare("SELECT name FROM pragma_table_info('plugin_actions')").all() as Array<{ name: string }>;
|
||||
expect(cols.some((c) => c.name === 'scope')).toBe(true);
|
||||
testDb.prepare("INSERT INTO plugin_actions (plugin_id, action_key, label, hint, danger, sort_order) VALUES ('m', 'k', 'K', NULL, 0, 0)").run();
|
||||
expect((testDb.prepare("SELECT scope FROM plugin_actions WHERE plugin_id='m'").get() as { scope: string }).scope).toBe('user');
|
||||
testDb.prepare("DELETE FROM plugin_actions WHERE plugin_id='m'").run();
|
||||
});
|
||||
});
|
||||
|
||||
describe('instance-scope actions (admin)', () => {
|
||||
function declareAction(pluginId: string, key: string, scope: 'user' | 'instance') {
|
||||
testDb.prepare('INSERT INTO plugin_actions (plugin_id, action_key, label, hint, danger, scope, sort_order) VALUES (?, ?, ?, NULL, 0, ?, 0)')
|
||||
.run(pluginId, key, key, scope);
|
||||
}
|
||||
const adminReq = { user: { id: 42 } } as unknown as Request;
|
||||
function controller(invoke = vi.fn(async () => ({ ok: true, message: 'pong' }))) {
|
||||
const rt = createPluginRuntime(new DatabaseService(dbConn));
|
||||
// isActive normally reflects the supervisor's live child map, which nothing here
|
||||
// spawns — so it's stubbed to read the same DB status the test itself flips,
|
||||
// mirroring what an actually-activated plugin would report.
|
||||
const isActive = (id: string) => !!testDb.prepare("SELECT 1 FROM plugins WHERE id = ? AND status = 'active'").get(id);
|
||||
const runtime = Object.assign(rt, { invokeAction: invoke, isActive }) as unknown as PluginRuntimeService;
|
||||
const c = new PluginsController(svc(), runtime, {} as unknown as PluginRegistryService, { isManaged: () => false } as unknown as RuntimeEnvService);
|
||||
return { c, invoke };
|
||||
}
|
||||
|
||||
beforeEach(() => { testDb.prepare('DELETE FROM plugin_actions').run(); });
|
||||
|
||||
it('ACT-ADM-001 — GET config lists the instance actions and none of the user ones', () => {
|
||||
install('p');
|
||||
declareAction('p', 'purge', 'instance');
|
||||
declareAction('p', 'testConnection', 'user');
|
||||
const { c } = controller();
|
||||
expect(c.getConfig('p').actions).toEqual([{ key: 'purge', label: 'purge', hint: undefined, danger: false, scope: 'instance' }]);
|
||||
});
|
||||
|
||||
it('ACT-ADM-002 — POST runs the action as the clicking admin in the instance scope', async () => {
|
||||
install('p');
|
||||
testDb.prepare("UPDATE plugins SET status = 'active', enabled = 1 WHERE id = 'p'").run();
|
||||
const { c, invoke } = controller();
|
||||
expect(await c.runAction('p', 'purge', adminReq)).toEqual({ ok: true, message: 'pong' });
|
||||
expect(invoke).toHaveBeenCalledWith('p', 'purge', 42, 'instance');
|
||||
});
|
||||
|
||||
it('ACT-ADM-003 — an inactive plugin answers 404 like the user route', async () => {
|
||||
install('p');
|
||||
const { c, invoke } = controller();
|
||||
await expect(c.runAction('p', 'purge', adminReq)).rejects.toMatchObject({ status: 404, response: { error: 'Plugin is not active' } });
|
||||
expect(invoke).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('ACT-ADM-004 — a refused key is a failed RESULT, not a server error', async () => {
|
||||
install('p');
|
||||
testDb.prepare("UPDATE plugins SET status = 'active', enabled = 1 WHERE id = 'p'").run();
|
||||
const { c } = controller(vi.fn(async () => { throw new Error('plugin p did not declare action "x" in scope instance'); }));
|
||||
expect(await c.runAction('p', 'x', adminReq)).toEqual({ ok: false, message: 'plugin p did not declare action "x" in scope instance' });
|
||||
});
|
||||
});
|
||||
|
||||
@@ -27,7 +27,7 @@ function ctrl() {
|
||||
updateUserConfig: vi.fn((_id: string, _uid: number, patch: Record<string, unknown>) => ({ ...patch, apiKey: '••••••••' })),
|
||||
} as unknown as PluginsService;
|
||||
// The controller now also takes the runtime (for settings-page actions).
|
||||
const runtime = { actionsOf: () => [], invokeAction: vi.fn() } as unknown as PluginRuntimeService;
|
||||
const runtime = { actionsOf: vi.fn(() => []), invokeAction: vi.fn(async () => ({ ok: true })) } as unknown as PluginRuntimeService;
|
||||
return { c: new PluginUserSettingsController(svc, runtime, new DatabaseService(dbConn)), svc, runtime };
|
||||
}
|
||||
|
||||
@@ -55,4 +55,12 @@ describe('PluginUserSettingsController', () => {
|
||||
getMock.mockReturnValue(undefined as never);
|
||||
expect(c.update('p', { config: { units: 'metric' } }, req(5))).toEqual({ config: {} });
|
||||
});
|
||||
|
||||
it('GET lists USER-scope actions only, and POST runs one as the caller in the user scope', async () => {
|
||||
const { c, runtime } = ctrl();
|
||||
c.get('p', req(5));
|
||||
expect(runtime.actionsOf).toHaveBeenCalledWith('p', 'user');
|
||||
expect(await c.runAction('p', 'sync', req(5))).toEqual({ ok: true });
|
||||
expect(runtime.invokeAction).toHaveBeenCalledWith('p', 'sync', 5, 'user');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -236,9 +236,9 @@ describe('PluginsController M2 endpoints', () => {
|
||||
});
|
||||
|
||||
it('get/update config delegate to the service (get carries the form fields, update the restart)', async () => {
|
||||
const rt = { activate: vi.fn(), deactivate: vi.fn(), isActive: vi.fn(), respawnIfActive: vi.fn(async () => false) } as never;
|
||||
const rt = { activate: vi.fn(), deactivate: vi.fn(), isActive: vi.fn(), respawnIfActive: vi.fn(async () => false), actionsOf: vi.fn(() => []) } as never;
|
||||
const c = new PluginsController(svc, rt, {} as never, envStub);
|
||||
expect(c.getConfig('x')).toEqual({ fields: [{ key: 'a' }], config: { a: 1 } });
|
||||
expect(c.getConfig('x')).toEqual({ fields: [{ key: 'a' }], config: { a: 1 }, actions: [] });
|
||||
expect(await c.updateConfig('x', { a: 2 })).toEqual({ config: { a: 2 }, restarted: false });
|
||||
});
|
||||
|
||||
|
||||
Reference in New Issue
Block a user