From 8f70951555b1876e6ea2f22ee3cf43b55cf4dec7 Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Wed, 14 Oct 2026 02:21:45 +0200 Subject: [PATCH] fix: give plugins their default settings and apply setting changes without a reload --- components/settings/plugins-settings.tsx | 3 +- .../plugin-host-bridge-origin.test.ts | 22 ++++++++ lib/__tests__/plugin-settings.test.ts | 30 +++++++++++ lib/__tests__/plugin-store.test.ts | 21 ++++++++ lib/plugin-sandbox/host-bridge.ts | 28 ++++++++++- lib/plugin-sandbox/protocol.ts | 4 ++ lib/plugin-sandbox/runtime.tsx | 34 ++++++++++++- lib/plugin-settings.ts | 50 +++++++++++++++++++ stores/plugin-store.ts | 21 ++++++-- 9 files changed, 204 insertions(+), 9 deletions(-) create mode 100644 lib/__tests__/plugin-settings.test.ts create mode 100644 lib/plugin-settings.ts diff --git a/components/settings/plugins-settings.tsx b/components/settings/plugins-settings.tsx index 625a98e7c..07bc35aa5 100644 --- a/components/settings/plugins-settings.tsx +++ b/components/settings/plugins-settings.tsx @@ -13,6 +13,7 @@ import { Button } from '@/components/ui/button'; import { ConfirmDialog } from '@/components/ui/confirm-dialog'; import { PluginPermissionList } from '@/components/plugins/plugin-permission-list'; import type { InstalledPlugin, PluginStatus, SettingFieldSchema } from '@/lib/plugin-types'; +import { effectiveSettings } from '@/lib/plugin-settings'; const STATUS_COLORS: Record = { installed: 'bg-muted text-muted-foreground', @@ -306,7 +307,7 @@ function PluginCard({ plugin, isExpanded, isForceEnabled, controlsDisabled, onTo key={key} fieldKey={key} schema={schema} - value={plugin.settings[key] ?? schema.default} + value={effectiveSettings(plugin)[key]} onChange={(value) => onUpdateSettings({ [key]: value })} /> ))} diff --git a/lib/__tests__/plugin-host-bridge-origin.test.ts b/lib/__tests__/plugin-host-bridge-origin.test.ts index 415b7a016..3be214421 100644 --- a/lib/__tests__/plugin-host-bridge-origin.test.ts +++ b/lib/__tests__/plugin-host-bridge-origin.test.ts @@ -48,3 +48,25 @@ describe('host bridge origin pin for untrusted sandboxes', () => { expect(dispatchApiCall).not.toHaveBeenCalled(); }); }); + +describe('init payload settings', () => { + it('hands a new iframe the settings with defaults filled in', async () => { + const p = { + ...plugin(), + settings: { size: 64 }, + settingsSchema: { + size: { type: 'number' as const, label: 'Size', default: 160 }, + mode: { type: 'select' as const, label: 'Mode', default: 'auto', options: ['auto', 'manual'] }, + }, + }; + const inst = createBackgroundInstance({ plugin: p, code: '', locale: 'en', tier: 'untrusted' }); + instances.push(inst); + const sent = vi.spyOn(inst.iframe.contentWindow!, 'postMessage'); + + post(inst.iframe.contentWindow, 'null', { type: 'sandbox-ready' }); + await new Promise((r) => setTimeout(r, 0)); + + const init = sent.mock.calls.map(([msg]) => msg as { type: string; payload?: { manifest: { settings: unknown } } }).find((m) => m.type === 'init'); + expect(init?.payload?.manifest.settings).toEqual({ size: 64, mode: 'auto' }); + }); +}); diff --git a/lib/__tests__/plugin-settings.test.ts b/lib/__tests__/plugin-settings.test.ts new file mode 100644 index 000000000..347ad0bc9 --- /dev/null +++ b/lib/__tests__/plugin-settings.test.ts @@ -0,0 +1,30 @@ +import { describe, it, expect } from 'vitest'; +import { effectiveSettings } from '../plugin-settings'; +import type { SettingFieldSchema } from '../plugin-types'; + +const schema: Record = { + size: { type: 'number', label: 'Size', default: 160, min: 16, max: 512 }, + mode: { type: 'select', label: 'Mode', default: 'auto', options: ['auto', 'manual'] }, + on: { type: 'boolean', label: 'On', default: true }, +}; + +describe('effectiveSettings', () => { + it('gives the plugin its defaults before the user saved anything', () => { + // Plugins used to get {} and had to repeat every default in code. + expect(effectiveSettings({ settings: {}, settingsSchema: schema })).toEqual({ size: 160, mode: 'auto', on: true }); + }); + + it('keeps valid saved values', () => { + expect(effectiveSettings({ settings: { size: 64, mode: 'manual', on: false }, settingsSchema: schema })) + .toEqual({ size: 64, mode: 'manual', on: false }); + }); + + it('replaces values the schema no longer accepts', () => { + const result = effectiveSettings({ settings: { size: 9999, mode: 'removed-option', on: 'yes', old: 1 }, settingsSchema: schema }); + expect(result).toEqual({ size: 512, mode: 'auto', on: true }); + }); + + it('passes settings through for a plugin without a schema', () => { + expect(effectiveSettings({ settings: { anything: 1 } })).toEqual({ anything: 1 }); + }); +}); diff --git a/lib/__tests__/plugin-store.test.ts b/lib/__tests__/plugin-store.test.ts index 5d3b4a56f..4aac4b9da 100644 --- a/lib/__tests__/plugin-store.test.ts +++ b/lib/__tests__/plugin-store.test.ts @@ -40,6 +40,10 @@ const mocks = vi.hoisted(() => ({ vi.mock('@/lib/browser-navigation', () => ({ apiFetch: (...args: unknown[]) => mocks.apiFetch(...args), })); +const bridge = vi.hoisted(() => ({ setSettings: vi.fn() })); +vi.mock('@/lib/plugin-sandbox/host-bridge', () => ({ + forEachInstance: (_id: string, fn: (i: { setSettings: typeof bridge.setSettings }) => void) => fn({ setSettings: bridge.setSettings }), +})); vi.mock('@/lib/plugin-sandbox/bundle-fetch', () => ({ downloadManagedBundle: (...args: unknown[]) => mocks.downloadManagedBundle(...args), })); @@ -334,3 +338,20 @@ describe('plugins the user installs', () => { expect(usePluginStore.getState().plugins[0].error).toBe('approval-denied'); }); }); + +describe('updatePluginSettings', () => { + it('sends the new settings to the running plugin right away', () => { + // Settings used to reach a plugin only after a reload. + resetStore(); + bridge.setSettings.mockClear(); + usePluginStore.setState({ plugins: [mockPlugin({ + id: 'p', settings: {}, + settingsSchema: { size: { type: 'number', label: 'Size', default: 160, min: 16, max: 512 } }, + })] }); + + usePluginStore.getState().updatePluginSettings('p', { size: 9000 }); + + expect(usePluginStore.getState().plugins[0].settings.size).toBe(512); + expect(bridge.setSettings).toHaveBeenCalledWith({ size: 512 }); + }); +}); diff --git a/lib/plugin-sandbox/host-bridge.ts b/lib/plugin-sandbox/host-bridge.ts index 5b3f67ca6..f0453d2c5 100644 --- a/lib/plugin-sandbox/host-bridge.ts +++ b/lib/plugin-sandbox/host-bridge.ts @@ -12,6 +12,7 @@ import { dispatchApiCall } from './host-api'; import { SANDBOX_PATH, SANDBOX_PRIVILEGED_PATH } from './protocol'; import { withBasePath } from '../browser-navigation'; import { snapshotHostTheme, type ThemeSnapshot } from './host-theme'; +import { effectiveSettings } from '../plugin-settings'; import type { SandboxToHost, HostToSandbox, InitMsg, InitPayload, } from './protocol'; @@ -81,6 +82,21 @@ export interface InitDoneInfo { shortcuts: Array<{ id: string; keys: string; label: string; category?: string }>; } +// ─── Live instances ────────────────────────────────────────── + +/** + * Every iframe currently running, background and slot alike, so a change + * that concerns a plugin (its settings, the app locale) reaches all of them. + */ +const liveInstances = new Set(); + +export function forEachInstance(pluginId: string | null, fn: (instance: SandboxInstance) => void): void { + for (const instance of liveInstances) { + if (pluginId !== null && instance.pluginId !== pluginId) continue; + try { fn(instance); } catch { /* one broken iframe must not stop the rest */ } + } +} + // ─── Sandbox instance ──────────────────────────────────────── export class SandboxInstance { @@ -165,6 +181,7 @@ export class SandboxInstance { this.listener = (ev) => this.onMessage(ev); window.addEventListener('message', this.listener); hostContainer.appendChild(this.iframe); + liveInstances.add(this); // Send init after the iframe runtime signals it's ready. this.readyPromise.then(() => { @@ -310,6 +327,12 @@ export class SandboxInstance { this.send({ type: 'theme-change', theme }); } + /** Push the plugin's current settings (see effectiveSettings). */ + setSettings(settings: Record): void { + if (this.destroyed) return; + this.send({ type: 'settings-change', settings }); + } + updateProps(props: Record): void { if (this.destroyed) return; // Stale references would leak if we kept growing the table without @@ -322,6 +345,7 @@ export class SandboxInstance { destroy(): void { if (this.destroyed) return; this.destroyed = true; + liveInstances.delete(this); window.removeEventListener('message', this.listener); this.iframe.remove(); for (const { reject } of this.pendingHookInvokes.values()) { @@ -348,7 +372,7 @@ export function createBackgroundInstance(opts: BackgroundOptions): SandboxInstan id: opts.plugin.id, version: opts.plugin.version, permissions: opts.plugin.permissions, - settings: { ...opts.plugin.settings }, + settings: effectiveSettings(opts.plugin), locales: opts.plugin.locales, httpOrigins: opts.plugin.httpOrigins, }, @@ -374,7 +398,7 @@ export function createSlotInstance(opts: SlotOptions): SandboxInstance { id: opts.plugin.id, version: opts.plugin.version, permissions: opts.plugin.permissions, - settings: { ...opts.plugin.settings }, + settings: effectiveSettings(opts.plugin), locales: opts.plugin.locales, httpOrigins: opts.plugin.httpOrigins, }, diff --git a/lib/plugin-sandbox/protocol.ts b/lib/plugin-sandbox/protocol.ts index f24f7d5ac..21c1d504e 100644 --- a/lib/plugin-sandbox/protocol.ts +++ b/lib/plugin-sandbox/protocol.ts @@ -177,6 +177,9 @@ export interface LocaleChangeMsg { type: 'locale-change'; locale: string; } /** Host → sandbox: the resolved theme changed; re-inject the slot's theme CSS. */ export interface ThemeChangeMsg { type: 'theme-change'; theme: ThemeSnapshot; } +/** Host → sandbox: the user changed this plugin's settings. */ +export interface SettingsChangeMsg { type: 'settings-change'; settings: Record; } + export interface PropsUpdateMsg { type: 'props-update'; props: Record; } export interface SlotShouldShowMsg { @@ -193,6 +196,7 @@ export type HostToSandbox = | HookInvokeMsg | LocaleChangeMsg | ThemeChangeMsg + | SettingsChangeMsg | PropsUpdateMsg | SlotShouldShowMsg; diff --git a/lib/plugin-sandbox/runtime.tsx b/lib/plugin-sandbox/runtime.tsx index cd377451e..03970b477 100644 --- a/lib/plugin-sandbox/runtime.tsx +++ b/lib/plugin-sandbox/runtime.tsx @@ -62,6 +62,19 @@ let bootDone = false; let readyPosted = false; const pendingApi = new Map void; reject: (err: Error) => void }>(); + +// The plugin's settings. One object for the iframe's lifetime, updated in +// place on 'settings-change', so code that kept a reference sees new values. +const settingsState: Record = {}; +const settingsListeners = new Set<(settings: Record) => void>(); + +function applySettings(next: Record): void { + for (const key of Object.keys(settingsState)) delete settingsState[key]; + Object.assign(settingsState, next); + for (const listener of settingsListeners) { + try { listener(settingsState); } catch (err) { console.error('[plugin] settings listener failed', err); } + } +} const pendingCallbacks = new Map void; reject: (err: Error) => void }>(); const hookHandlers: Record unknown> = {}; @@ -215,11 +228,18 @@ interface PluginKeywordCounts { } function buildPluginApi(manifest: PluginManifest) { + applySettings({ ...manifest.settings }); return { plugin: { id: manifest.id, version: manifest.version, - settings: { ...manifest.settings }, + /** Current settings, defaults filled in. Updated in place when the user changes them. */ + settings: settingsState, + /** Called with the new settings whenever the user changes them. Returns an unsubscribe function. */ + onSettingsChange: (listener: (settings: Record) => void) => { + settingsListeners.add(listener); + return () => { settingsListeners.delete(listener); }; + }, }, crypto: { getPublicKeys: () => callApi('crypto.getPublicKeys', []), @@ -634,10 +654,18 @@ function bootSlot(payload: SlotInit): void { const SlotShell = () => { const wrapRef = React.useRef(null); const [props, setProps] = React.useState(currentProps); + const [, setSettingsVersion] = React.useState(0); React.useEffect(() => { propsListeners.add(setProps); return () => { propsListeners.delete(setProps); }; }, []); + // Re-render the slot when its settings change, so a component that reads + // api.plugin.settings while rendering shows the new values. + React.useEffect(() => { + const bump = () => setSettingsVersion((v) => v + 1); + settingsListeners.add(bump); + return () => { settingsListeners.delete(bump); }; + }, []); React.useEffect(() => { if (!wrapRef.current) return; let lastHeight = -1; @@ -759,6 +787,10 @@ function handleHostMessage(ev: MessageEvent): void { applyHostTheme(msg.theme); break; + case 'settings-change': + applySettings(msg.settings ?? {}); + break; + case 'props-update': slotPropsUpdater?.(msg.props ?? {}); break; diff --git a/lib/plugin-settings.ts b/lib/plugin-settings.ts new file mode 100644 index 000000000..0453b6046 --- /dev/null +++ b/lib/plugin-settings.ts @@ -0,0 +1,50 @@ +// What a plugin's settings actually are: the manifest defaults, overlaid +// with what the user saved, each value checked against the schema. Used by +// the settings page and by everything that hands settings to the plugin, +// so the two can't disagree. + +import type { InstalledPlugin, SettingFieldSchema } from './plugin-types'; + +function coerce(field: SettingFieldSchema, value: unknown): unknown { + switch (field.type) { + case 'boolean': + return typeof value === 'boolean' ? value : field.default; + case 'string': + return typeof value === 'string' ? value : field.default; + case 'select': + return typeof value === 'string' && Array.isArray(field.options) && field.options.includes(value) + ? value + : field.default; + case 'number': { + if (typeof value !== 'number' || !Number.isFinite(value)) return field.default; + if (field.min !== undefined && value < field.min) return field.min; + if (field.max !== undefined && value > field.max) return field.max; + return value; + } + default: + return field.default; + } +} + +/** + * Settings as the plugin should see them. Without a schema the stored + * values pass through unchanged (older plugins read free-form settings). + * With one, every declared field has a valid value and nothing else is kept, + * so a field removed in an update or an option that no longer exists can't + * linger. + */ +export function effectiveSettings(plugin: Pick): Record { + const stored = plugin.settings ?? {}; + const schema = plugin.settingsSchema; + if (!schema || Object.keys(schema).length === 0) return { ...stored }; + const out: Record = {}; + for (const [key, field] of Object.entries(schema)) { + out[key] = key in stored ? coerce(field, stored[key]) : field.default; + } + return out; +} + +/** A single value as it will be stored, or the field's default if invalid. */ +export function coerceSetting(field: SettingFieldSchema, value: unknown): unknown { + return coerce(field, value); +} diff --git a/stores/plugin-store.ts b/stores/plugin-store.ts index 28fcd32fd..3917a5358 100644 --- a/stores/plugin-store.ts +++ b/stores/plugin-store.ts @@ -15,6 +15,8 @@ import { packageHash, type ValidatedPluginManifest } from '@/lib/extension-packa import JSZip from 'jszip'; import { sha256Hex } from '@/lib/plugin-sandbox/bundle-integrity'; import { downloadManagedBundle } from '@/lib/plugin-sandbox/bundle-fetch'; +import { forEachInstance } from '@/lib/plugin-sandbox/host-bridge'; +import { coerceSetting, effectiveSettings } from '@/lib/plugin-settings'; import { usePolicyStore } from '@/stores/policy-store'; import { apiFetch } from '@/lib/browser-navigation'; import { IMPLICIT_PERMISSIONS } from '@/lib/plugin-types'; @@ -238,11 +240,20 @@ export const usePluginStore = create()( updatePluginSettings: (id: string, settings: Record) => { const { plugins } = get(); - set({ - plugins: plugins.map(p => - p.id === id ? { ...p, settings: { ...p.settings, ...settings } } : p - ), - }); + const plugin = plugins.find(p => p.id === id); + if (!plugin) return; + // Keep only values the schema accepts (numbers within min/max, an + // existing select option), so what is stored is what the plugin gets. + const schema = plugin.settingsSchema ?? {}; + const accepted: Record = {}; + for (const [key, value] of Object.entries(settings)) { + accepted[key] = schema[key] ? coerceSetting(schema[key], value) : value; + } + const updated = { ...plugin, settings: { ...plugin.settings, ...accepted } }; + set({ plugins: plugins.map(p => (p.id === id ? updated : p)) }); + // Running iframes get the change right away; no reload needed. + const next = effectiveSettings(updated); + forEachInstance(id, (instance) => instance.setSettings(next)); }, setPluginStatus: (id: string, status: PluginStatus, error?: string) => {