diff --git a/docs/plugin-foundation/PLUGIN-AUTHORING.md b/docs/plugin-foundation/PLUGIN-AUTHORING.md index b2236b6e..5d2d10e4 100644 --- a/docs/plugin-foundation/PLUGIN-AUTHORING.md +++ b/docs/plugin-foundation/PLUGIN-AUTHORING.md @@ -50,6 +50,18 @@ overlays: [{ }] ``` +> **The host owns placement — render RELATIVE content, don't self-position.** Your +> `render()` output is dropped into a host-positioned, ~300px-wide slot that +> expands upward from the bottom-left, clear of the canvas's zoom/dark-mode +> controls and height-capped to the canvas stage (a tall overlay scrolls within +> the slot rather than covering the top toolbar). Do **not** set `position: +> fixed`/`absolute` with your own `top`/`left`/`bottom` on the root — that escapes +> the safe zone and can cover app controls (the host does not sandbox this; it's a +> convention). Style the panel's look (background, border, padding) but let the +> host place and size it — no need to set a shadow, the host slot provides one. At +> most one overlay is open at a time, and it shares its slot with the plugin +> manager. + ### Export slots — action-time **void** `onSelect(ctx)` ```js diff --git a/web/src/components/PluginErrorBoundary.jsx b/web/src/components/PluginErrorBoundary.jsx index 3dc9e8a8..9713fcad 100644 --- a/web/src/components/PluginErrorBoundary.jsx +++ b/web/src/components/PluginErrorBoundary.jsx @@ -37,9 +37,10 @@ export default class PluginErrorBoundary extends React.Component {
@@ -47,18 +48,36 @@ export default class PluginErrorBoundary extends React.Component { Plugin “{this.props.label}” unavailable
{detail}
- {this.props.onClose && ( - - )} +
+ {this.props.onClose && ( + + )} + {/* Quick-eject: a render-crashed plugin can be turned off for good. + The host filters on the disabled set, so the crashed slot + disappears on the re-render and stays gone across reloads. */} + {this.props.onDisable && ( + + )} +
); } diff --git a/web/src/components/PluginOverlayHost.jsx b/web/src/components/PluginOverlayHost.jsx index 1f896035..92ed407e 100644 --- a/web/src/components/PluginOverlayHost.jsx +++ b/web/src/components/PluginOverlayHost.jsx @@ -2,7 +2,13 @@ // state (which overlay is open, if any) so the canvas monolith stays additive: // the canvas hands it one `api` capability bag (#167's CanvasApi) and this host // does the rest — launcher buttons, per-plugin context minting, error -// isolation, one-overlay-at-a-time enforcement. +// isolation, one-overlay-at-a-time enforcement, and PLACEMENT: overlays render +// as relative content into a host-positioned safe zone (Option A) — anchored at +// left:58 clear of the native zoom/dark-mode column, and height-capped to the +// canvas stage. So a well-behaved (relative) plugin can't cover those controls +// or collide with the manager. (It does NOT sandbox: a plugin that sets its own +// position:fixed/absolute can still escape — a documented convention, not a +// clip.) // // The version gate + its plugin-naming console.warn live in loadFeaturePlugins // (registry.ts, via the pure selectRenderablePlugins) — this host consumes that @@ -12,19 +18,32 @@ import React, { useEffect, useState } from "react"; import { loadFeaturePlugins } from "../lib/plugins/registry.js"; import { mintPluginCtx } from "../lib/plugins/host.js"; +import { useDisabledPluginIds } from "../lib/plugins/useDisabledPlugins.js"; +import { setPluginDisabled } from "../lib/plugins/pluginPrefs.js"; import PluginErrorBoundary from "./PluginErrorBoundary.jsx"; const launcherStyle = { + flexShrink: 0, // launchers never shrink — only the panel gives way when tall padding: "6px 10px", border: "1px solid var(--ink-faint)", background: "var(--paper-bright)", color: "var(--ink)", cursor: "pointer", fontSize: 12, fontWeight: 600, boxShadow: "var(--shadow-1)", textAlign: "left", }; +// The host-owned width of the panel slot (overlay + manager). Plugins render +// RELATIVE content that fills this — they don't pick their own size or position. +const PANEL_WIDTH = 300; + export default function PluginOverlayHost({ api, onActionError }) { const [plugins, setPlugins] = useState([]); // Single open slot — one overlay at a time in v1 (concurrent overlays are // explicitly deferred). Value is the "pluginId::overlayId" key, or null. const [openKey, setOpenKey] = useState(null); + // Manager popover open/closed. Independent of an overlay being open — the + // manager is the ONLY re-enable path, so it must be reachable even when every + // plugin is disabled (no launchers, no open overlay). + const [showManager, setShowManager] = useState(false); + // Per-user disabled set (reactive: re-renders when toggled here or elsewhere). + const disabled = useDisabledPluginIds(); // Load the in-tree feature descriptors once. `live` guards a resolve that // lands after unmount (and makes StrictMode's double-mount harmless: the first @@ -38,55 +57,154 @@ export default function PluginOverlayHost({ api, onActionError }) { return () => { live = false; }; }, []); - const slots = plugins.flatMap((plugin) => - plugin.overlays.map((overlay) => ({ - plugin, - overlay, - key: `${plugin.id}::${overlay.id}`, - })), - ); - if (slots.length === 0) return null; + // If the currently-open overlay's plugin gets disabled (here or in another + // tab), close it — otherwise re-enabling later would silently reopen the old + // overlay. `openSlot.find` already renders nothing once filtered, but the key + // must be cleared too. + useEffect(() => { + if (openKey && disabled.has(openKey.split("::")[0])) setOpenKey(null); + }, [disabled, openKey]); + + // Launcher/overlay slots come ONLY from ENABLED plugins — a disabled plugin + // contributes no launcher and no overlay. The manager below still iterates the + // FULL `plugins` set so a disabled plugin can be re-enabled. + const slots = plugins + .filter((p) => !disabled.has(p.id)) + .flatMap((plugin) => + plugin.overlays.map((overlay) => ({ + plugin, + overlay, + key: `${plugin.id}::${overlay.id}`, + })), + ); + + // Gate on the FULL loaded set, not the (filtered) slots: the manager must + // render whenever ANY plugin is loaded — including export-only plugins with no + // overlay, and the all-disabled case where there are zero slots but the user + // still needs a way back in. + if (plugins.length === 0) return null; const close = () => setOpenKey(null); const openSlot = slots.find((s) => s.key === openKey) ?? null; + // Option A — the HOST owns placement. All plugin UI lives in ONE bottom-anchored + // column at left:58 (clear of the canvas's native zoom/dark-mode column at + // left:14). The panel slot (overlay OR manager) sits ABOVE the launchers and + // expands upward; manager and overlay are MUTUALLY EXCLUSIVE, so they can't + // cover each other, and a plugin's overlay renders as RELATIVE content into a + // host-sized box — it can't self-position over the canvas. + const openOverlay = (key) => { setShowManager(false); setOpenKey((v) => (v === key ? null : key)); }; + const toggleManager = () => { setOpenKey(null); setShowManager((v) => !v); }; + return ( - <> -
- {slots.map(({ overlay, key }) => ( - - ))} -
+ {openSlot.overlay.render({ + ctx: mintPluginCtx(api, openSlot.plugin.id, onActionError), + onClose: close, + })} + + + )} - {/* At most ONE overlay is rendered — openSlot is a single slot or null, so - one-overlay-at-a-time is enforced structurally, not just visually. Each - render-time slot is wrapped in its own error boundary: a plugin that - throws in RENDER degrades to a "feature unavailable" notice and the - canvas survives. Action-time throws (a plugin's own onClick calling a - ctx command) can't reach the boundary; `onActionError`, threaded into - the minted ctx, contains + surfaces those instead. */} - {openSlot && ( - - {openSlot.overlay.render({ - ctx: mintPluginCtx(api, openSlot.plugin.id, onActionError), - onClose: close, + {/* Manager — shares the panel slot with the overlay (mutually exclusive). + Always reachable whenever any plugin is loaded, so re-enable works even + with every plugin disabled. Lists the FULL set with an Enable/Disable + toggle each. */} + {showManager && ( +
+ {plugins.map((plugin) => { + const off = disabled.has(plugin.id); + return ( +
+ + {plugin.id} + + +
+ ); })} - +
)} - + + {/* Launchers + the manager toggle — the stable button group at the bottom + (nearest the corner); the panel above expands upward. */} + {slots.map(({ overlay, key }) => ( + + ))} + + ); } diff --git a/web/src/features/takeoff-notes/plugin.jsx b/web/src/features/takeoff-notes/plugin.jsx index 85739009..7f8041d4 100644 --- a/web/src/features/takeoff-notes/plugin.jsx +++ b/web/src/features/takeoff-notes/plugin.jsx @@ -62,9 +62,11 @@ function NotesPanel({ ctx, onClose }) { return (
`, a same-tab CustomEvent, and a cross-tab `storage` +// listener; a subscribe fn returns its own unsubscribe. +// +// HONEST LIMITATION: disable ejects a plugin from ACTIVATION and RENDER (no +// launcher, no overlay, no export item). It does NOT skip the module's IMPORT — +// the feature glob resolves every thunk to learn each plugin's id, so ids are +// only known POST-import. A plugin that misbehaves purely at import-eval time is +// still contained by the loader's existing try/catch (registry.ts resolveModules +// logs + skips a throwing thunk), but its module still evaluates. A stronger +// skip-the-chunk eject (persisting ids to gate the import itself) is deferred. + +const KEY = "opentakeoff_plugins_disabled"; +const EVT = "opentakeoff:plugins-disabled"; + +// Reads are done at CALL time behind typeof-guards so this module imports +// cleanly under node (the unit test), where `localStorage`/`window` are absent +// until the test stubs them onto globalThis. +function readStore() { + if (typeof localStorage === "undefined") return null; + try { + return localStorage.getItem(KEY); + } catch { + return null; // private mode / access denied — treat as no stored choice + } +} + +function writeStore(ids) { + if (typeof localStorage === "undefined") return; + try { + localStorage.setItem(KEY, JSON.stringify(ids)); + } catch { + /* private mode — session-only, best effort */ + } +} + +/** The set of disabled plugin ids. Tolerates a missing or malformed value — + * never throws, returns an empty set on any parse failure. */ +export function getDisabledPluginIds() { + const raw = readStore(); + if (!raw) return new Set(); + try { + const parsed = JSON.parse(raw); + if (!Array.isArray(parsed)) return new Set(); + return new Set(parsed.filter((id) => typeof id === "string")); + } catch { + return new Set(); // malformed JSON — behave as if nothing is disabled + } +} + +/** Is this plugin id currently disabled? */ +export function isPluginDisabled(id) { + return getDisabledPluginIds().has(id); +} + +/** Add or remove `id` from the disabled set, persist, then notify live UIs + * (same tab via CustomEvent; other tabs via the browser's own `storage`). */ +export function setPluginDisabled(id, disabled) { + const ids = getDisabledPluginIds(); + if (disabled) ids.add(id); + else ids.delete(id); + writeStore([...ids]); + if (typeof window !== "undefined") { + window.dispatchEvent(new CustomEvent(EVT, { detail: [...ids] })); + } +} + +/** Subscribe to disabled-set changes from this tab (CustomEvent) OR another tab + * (cross-tab `storage`). Returns the unsubscribe fn, so it can be a useEffect + * body directly. */ +export function onDisabledPluginsChange(fn) { + if (typeof window === "undefined") return () => {}; + const onEvt = () => fn(getDisabledPluginIds()); + const onStorage = (e) => { + if (e.key === KEY) fn(getDisabledPluginIds()); + }; + window.addEventListener(EVT, onEvt); + window.addEventListener("storage", onStorage); + return () => { + window.removeEventListener(EVT, onEvt); + window.removeEventListener("storage", onStorage); + }; +} diff --git a/web/src/lib/plugins/useDisabledPlugins.js b/web/src/lib/plugins/useDisabledPlugins.js new file mode 100644 index 00000000..7e1cb3dc --- /dev/null +++ b/web/src/lib/plugins/useDisabledPlugins.js @@ -0,0 +1,13 @@ +// Reactive read of the per-user disabled-plugin set. Both render-time consumers +// (PluginOverlayHost, TakeoffCanvas's export filter) use this hook so the +// subscribe/unsubscribe wiring lives in exactly one place. Returns the current +// Set and re-renders when the set changes (this tab or another). + +import { useEffect, useState } from "react"; +import { getDisabledPluginIds, onDisabledPluginsChange } from "./pluginPrefs.js"; + +export function useDisabledPluginIds() { + const [disabled, setDisabled] = useState(getDisabledPluginIds); + useEffect(() => onDisabledPluginsChange(setDisabled), []); + return disabled; +} diff --git a/web/src/pages/TakeoffCanvas.jsx b/web/src/pages/TakeoffCanvas.jsx index b68796f5..b38bf52d 100644 --- a/web/src/pages/TakeoffCanvas.jsx +++ b/web/src/pages/TakeoffCanvas.jsx @@ -64,6 +64,8 @@ import PluginOverlayHost from "../components/PluginOverlayHost.jsx"; // #168 import { downloadText as pluginDownloadText } from "../lib/totals.js"; // #168 — the ctx.download impl handed to plugins (additive) import { loadFeaturePlugins } from "../lib/plugins/registry.js"; // #169 — export-slot plugins for the report menu (additive) import { buildExportItems } from "../lib/plugins/exportItems.js"; // #169 — pre-bound, dispatch-isolated export items (additive) +import { useDisabledPluginIds } from "../lib/plugins/useDisabledPlugins.js"; // per-user plugin disable — reactive disabled set (additive) +import { setPluginDisabled } from "../lib/plugins/pluginPrefs.js"; // per-user plugin disable — eject from the action-error banner (additive) import AiSettings from "../components/AiSettings.jsx"; import { AGENT_TOOL_DEFS, executeAgentTool, agentScaleGate } from "../lib/agentTools.js"; import { runAgentLoop } from "../lib/agentLoop.js"; @@ -483,10 +485,14 @@ export default function TakeoffCanvas() { // in a try/catch (a React boundary can't catch a ToolMenu onClick throw), and // a caught throw surfaces this non-fatal notice instead of crashing the report. const [exportPlugins, setExportPlugins] = useState([]); + // Per-user disabled set (reactive): filters export slots below, so a disabled + // plugin contributes NO export item and it re-renders when toggled. + const disabledPlugins = useDisabledPluginIds(); // One shared non-fatal notice for any dispatch-time plugin ACTION fault — // a throwing export onSelect (#169) OR an overlay plugin's command throwing // from its own event handler (#168 I-1). Both surface here; the banner below - // renders it. + // renders it. Carries the offending `pluginId` too, so the banner can offer a + // "Disable plugin" eject. null = no error. const [pluginActionError, setPluginActionError] = useState(null); useEffect(() => { let live = true; @@ -500,10 +506,13 @@ export default function TakeoffCanvas() { // must close over the CURRENT api — a mount snapshot would make plugin reads // stale. The build is cheap (one closure per export slot). const extraExportItems = buildExportItems( - exportPlugins, + exportPlugins.filter((p) => !disabledPlugins.has(p.id)), pluginApi, (pluginId, exportId, err) => - setPluginActionError(`Export “${pluginId}::${exportId}” failed: ${err instanceof Error ? err.message : String(err)}`), + setPluginActionError({ + pluginId, + message: `Export “${pluginId}::${exportId}” failed: ${err instanceof Error ? err.message : String(err)}`, + }), ); const containerRef = useRef(null); @@ -5813,6 +5822,21 @@ export default function TakeoffCanvas() {
+ {/* #168 — opt-in plugin overlays. INSIDE the stage (a definite-height, + overflow:hidden box below the top chrome) so the overlay slot's + stage-relative height cap actually clamps and a tall overlay scrolls + within the stage instead of spilling over the toolbar. Renders nothing + when no feature folders are present. onActionError surfaces a plugin's + action-time command throw into the shared notice banner below. */} + + setPluginActionError({ + pluginId, + message: `Plugin “${pluginId}” action failed: ${err instanceof Error ? err.message : String(err)}`, + })} + /> + {/* status line — the transient message bar (was the right end of the old conditions bar): floats bottom-center over the canvas, never blocks input */} {commitMsg && ( @@ -6114,7 +6138,14 @@ export default function TakeoffCanvas() { report panel (z 60–70) so it is visible while the report is open. */} {pluginActionError && (
- {pluginActionError} + {pluginActionError.message} + {/* Quick-eject: an action-time fault names its plugin, so the banner + can turn it off for good (filtered out of export slots + overlays) + and clear the stale notice. */} + {pluginActionError.pluginId && ( + + )}
@@ -6134,17 +6165,6 @@ export default function TakeoffCanvas() { (the Agent panel links here; closing re-renders, so `configured` re-reads immediately). */} {showAiSettings && setShowAiSettings(false)} />} - - {/* #168 — opt-in plugin overlays. Renders nothing when no feature folders - are present (public core ships none); each overlay is version-gated and - error-isolated inside the host. `onActionError` surfaces a plugin's - action-time command throw (uncatchable by the render boundary) into the - shared notice banner above. */} - - setPluginActionError(`Plugin “${pluginId}” action failed: ${err instanceof Error ? err.message : String(err)}`)} - /> ); } diff --git a/web/test/pluginPrefs.test.ts b/web/test/pluginPrefs.test.ts new file mode 100644 index 00000000..a05d7e12 --- /dev/null +++ b/web/test/pluginPrefs.test.ts @@ -0,0 +1,96 @@ +// pluginPrefs is host-side (NOT the frozen core): a localStorage-backed disabled +// set with theme.js-style reactivity. Tested under node with stubbed globals — +// a Map-backed fake localStorage and an EventTarget-backed fake window — proving +// add/remove round-trip, isPluginDisabled, malformed-JSON tolerance, and that a +// write survives a re-read (persistence). Node 24 provides EventTarget + +// CustomEvent as globals, so `new EventTarget()` supplies add/remove/dispatch. +import { test, beforeEach } from "node:test"; +import assert from "node:assert/strict"; +import { + getDisabledPluginIds, + isPluginDisabled, + setPluginDisabled, + onDisabledPluginsChange, +} from "../src/lib/plugins/pluginPrefs.js"; + +const KEY = "opentakeoff_plugins_disabled"; + +// Minimal localStorage shim — the only two methods the module reaches for. +function fakeLocalStorage() { + const map = new Map(); + return { + getItem: (k: string) => (map.has(k) ? map.get(k)! : null), + setItem: (k: string, v: string) => void map.set(k, v), + raw: map, + }; +} + +beforeEach(() => { + (globalThis as Record).localStorage = fakeLocalStorage(); + (globalThis as Record).window = new EventTarget(); +}); + +test("empty by default (no stored value)", () => { + assert.deepEqual([...getDisabledPluginIds()], []); + assert.equal(isPluginDisabled("takeoff-notes"), false); +}); + +test("add/remove round-trip via setPluginDisabled", () => { + setPluginDisabled("takeoff-notes", true); + assert.equal(isPluginDisabled("takeoff-notes"), true); + assert.deepEqual([...getDisabledPluginIds()], ["takeoff-notes"]); + + setPluginDisabled("scope-summary", true); + assert.deepEqual([...getDisabledPluginIds()].sort(), ["scope-summary", "takeoff-notes"]); + + setPluginDisabled("takeoff-notes", false); + assert.equal(isPluginDisabled("takeoff-notes"), false); + assert.deepEqual([...getDisabledPluginIds()], ["scope-summary"]); +}); + +test("disabling the same id twice is idempotent (no duplicates)", () => { + setPluginDisabled("p", true); + setPluginDisabled("p", true); + assert.deepEqual([...getDisabledPluginIds()], ["p"]); +}); + +test("persists across a fresh read (write → re-read same backing store)", () => { + setPluginDisabled("p", true); + // Simulate a reload: a brand-new getDisabledPluginIds call reads the same + // localStorage backing that setPluginDisabled wrote. + assert.equal(isPluginDisabled("p"), true); + assert.equal( + (globalThis as { localStorage: { getItem(k: string): string | null } }).localStorage.getItem(KEY), + JSON.stringify(["p"]), + ); +}); + +test("malformed JSON tolerated → empty set, never throws", () => { + (globalThis as { localStorage: { setItem(k: string, v: string): void } }) + .localStorage.setItem(KEY, "{not json"); + assert.deepEqual([...getDisabledPluginIds()], []); + assert.equal(isPluginDisabled("anything"), false); +}); + +test("non-array JSON tolerated → empty set", () => { + (globalThis as { localStorage: { setItem(k: string, v: string): void } }) + .localStorage.setItem(KEY, JSON.stringify({ takeoff: true })); + assert.deepEqual([...getDisabledPluginIds()], []); +}); + +test("non-string members are filtered out", () => { + (globalThis as { localStorage: { setItem(k: string, v: string): void } }) + .localStorage.setItem(KEY, JSON.stringify(["ok", 3, null, "also-ok"])); + assert.deepEqual([...getDisabledPluginIds()].sort(), ["also-ok", "ok"]); +}); + +test("onDisabledPluginsChange fires on setPluginDisabled and unsubscribes", () => { + const seen: string[][] = []; + const off = onDisabledPluginsChange((set: Set) => seen.push([...set])); + setPluginDisabled("p", true); + assert.deepEqual(seen.at(-1), ["p"]); + off(); + setPluginDisabled("q", true); + // No further notification after unsubscribe. + assert.deepEqual(seen.at(-1), ["p"]); +});