diff --git a/frontend/src/features/plugins/PluginLayoutCellView.test.tsx b/frontend/src/features/plugins/PluginLayoutCellView.test.tsx index a3d20d4..71bc41e 100644 --- a/frontend/src/features/plugins/PluginLayoutCellView.test.tsx +++ b/frontend/src/features/plugins/PluginLayoutCellView.test.tsx @@ -2,7 +2,7 @@ * F4 — `PluginLayoutCellView` (ticket #43, carnet §10 F4 acceptance criteria: * "rendu composant mock, state roundtrip, fallback sans mutation du layout"). */ -import { describe, expect, it } from "vitest"; +import { describe, expect, it, vi } from "vitest"; import { render, screen, fireEvent } from "@testing-library/react"; import type { CustomPluginLayoutCell } from "@/domain"; @@ -39,7 +39,11 @@ function MockLayoutComponent(props: PluginLayoutProps) { ); } -function stubPlugin(): LoadedPlugin { +function ThrowingLayoutComponent(_props: PluginLayoutProps): JSX.Element { + throw new Error("plugin layout render failed"); +} + +function stubPlugin(component = MockLayoutComponent): LoadedPlugin { const contributes = { menus: [], menuItems: [], @@ -47,7 +51,7 @@ function stubPlugin(): LoadedPlugin { mcpServers: [], }; const layouts = new PluginLayoutRegistry("dev.acme.gitgraph", new Set(["dev.acme.gitgraph.layout"])); - layouts.register({ type: "dev.acme.gitgraph.layout", component: MockLayoutComponent }); + layouts.register({ type: "dev.acme.gitgraph.layout", component }); return { pluginId: "dev.acme.gitgraph", displayName: "Git Graph", @@ -110,4 +114,18 @@ describe("PluginLayoutCellView", () => { // The cell object itself is untouched — the domain identity/state survive. expect(testCell).toEqual(cell()); }); + + it("isolates a plugin layout render failure to the plugin cell fallback", () => { + const consoleError = vi.spyOn(console, "error").mockImplementation(() => {}); + try { + const registry = new PluginRuntimeRegistry(); + registry.add(stubPlugin(ThrowingLayoutComponent)); + renderCell(registry); + + expect(screen.getByText("Layout indisponible")).toBeTruthy(); + expect(screen.queryByTestId("state")).toBeNull(); + } finally { + consoleError.mockRestore(); + } + }); }); diff --git a/frontend/src/features/plugins/PluginLayoutCellView.tsx b/frontend/src/features/plugins/PluginLayoutCellView.tsx index beafa59..c06d7bc 100644 --- a/frontend/src/features/plugins/PluginLayoutCellView.tsx +++ b/frontend/src/features/plugins/PluginLayoutCellView.tsx @@ -10,6 +10,8 @@ * calls from inside a plugin component). */ +import { Component, type ErrorInfo, type ReactNode } from "react"; + import type { CustomPluginLayoutCell, PluginAdmin } from "@/domain"; import { useGateways } from "@/app/di"; import { usePluginRuntime } from "./PluginRuntimeProvider"; @@ -25,6 +27,38 @@ interface PluginLayoutCellViewProps { onChooseAnotherLayout: () => void; } +interface PluginLayoutErrorBoundaryProps { + resetKey: string; + fallback: ReactNode; + children: ReactNode; +} + +class PluginLayoutErrorBoundary extends Component< + PluginLayoutErrorBoundaryProps, + { hasError: boolean } +> { + state = { hasError: false }; + + static getDerivedStateFromError(): { hasError: boolean } { + return { hasError: true }; + } + + componentDidCatch(error: Error, errorInfo: ErrorInfo): void { + console.error("[plugin-layout] render failed", error, errorInfo); + } + + componentDidUpdate(prevProps: PluginLayoutErrorBoundaryProps): void { + if (prevProps.resetKey !== this.props.resetKey && this.state.hasError) { + this.setState({ hasError: false }); + } + } + + render(): ReactNode { + if (this.state.hasError) return this.props.fallback; + return this.props.children; + } +} + export function PluginLayoutCellView({ projectId, cell, @@ -36,11 +70,11 @@ export function PluginLayoutCellView({ const { registry } = usePluginRuntime(); const gateways = useGateways(); const availability = resolvePluginLayoutAvailability(registry, cell, installedPlugins); + const providerDisplayName = + registry.get(cell.pluginId)?.displayName ?? + installedPlugins?.find((p) => p.id === cell.pluginId)?.displayName; if (availability !== "available") { - const providerDisplayName = - registry.get(cell.pluginId)?.displayName ?? - installedPlugins?.find((p) => p.id === cell.pluginId)?.displayName; return ( ); + return ( + + + + ); } diff --git a/frontend/src/features/plugins/PluginLayoutSelectorSection.test.tsx b/frontend/src/features/plugins/PluginLayoutSelectorSection.test.tsx index f026c67..e66be38 100644 --- a/frontend/src/features/plugins/PluginLayoutSelectorSection.test.tsx +++ b/frontend/src/features/plugins/PluginLayoutSelectorSection.test.tsx @@ -12,6 +12,7 @@ import { PluginRuntimeRegistry, type LoadedPlugin, } from "@/plugins/runtime"; +import type { PluginContributionDto } from "@/domain"; import { PluginLayoutSelectorSection, listPluginLayoutChoices } from "./PluginLayoutSelectorSection"; function stubPlugin(pluginId: string, displayName: string, layoutType: string): LoadedPlugin { @@ -66,4 +67,32 @@ describe("listPluginLayoutChoices / PluginLayoutSelectorSection", () => { fireEvent.click(screen.getByRole("menuitem", { name: /One layout/ })); expect(selected).toBe("dev.acme.one.layout"); }); + + it("skips malformed layout contributions instead of throwing during sorting", () => { + const registry = new PluginRuntimeRegistry(); + const contributes = { + menus: [], + menuItems: [], + layouts: [ + { type: "dev.acme.good", label: "Good layout", component: "X" }, + { type: "dev.acme.no-label", label: undefined, component: "X" }, + { type: undefined, label: "No type", component: "X" }, + ] as unknown as PluginContributionDto["layouts"], + mcpServers: [], + }; + registry.add({ + pluginId: "dev.acme.bad", + displayName: undefined as unknown as string, + contributes, + commands: new PluginCommandRegistry("dev.acme.bad", new Set()), + layouts: new PluginLayoutRegistry("dev.acme.bad", new Set()), + menu: new PluginMenuRegistry("dev.acme.bad"), + dispose: async () => {}, + }); + + const choices = listPluginLayoutChoices(registry); + expect(choices).toHaveLength(1); + expect(choices[0].layout.label).toBe("Good layout"); + expect(choices[0].pluginDisplayName).toBe("dev.acme.bad"); + }); }); diff --git a/frontend/src/features/plugins/PluginLayoutSelectorSection.tsx b/frontend/src/features/plugins/PluginLayoutSelectorSection.tsx index fb740e5..7e364ee 100644 --- a/frontend/src/features/plugins/PluginLayoutSelectorSection.tsx +++ b/frontend/src/features/plugins/PluginLayoutSelectorSection.tsx @@ -26,15 +26,44 @@ export interface PluginLayoutChoice { layout: PluginLayoutContribution; } +function nonEmptyString(value: unknown): string | undefined { + return typeof value === "string" && value.trim().length > 0 ? value : undefined; +} + +function finiteOrder(value: unknown): number { + return typeof value === "number" && Number.isFinite(value) ? value : 0; +} + +function compareText(a: unknown, b: unknown): number { + return String(a ?? "").localeCompare(String(b ?? "")); +} + export function listPluginLayoutChoices(registry: PluginRuntimeRegistry): PluginLayoutChoice[] { return registry .layoutContributions() - .map(({ pluginId, pluginDisplayName, layout }) => ({ pluginId, pluginDisplayName, layout })) + .flatMap(({ pluginId, pluginDisplayName, layout }) => { + const type = nonEmptyString(layout.type); + const label = nonEmptyString(layout.label); + if (!type || !label) return []; + return [ + { + pluginId, + pluginDisplayName: nonEmptyString(pluginDisplayName) ?? pluginId, + layout: { + ...layout, + type, + label, + order: finiteOrder(layout.order), + icon: nonEmptyString(layout.icon), + }, + }, + ]; + }) .sort( (a, b) => (a.layout.order ?? 0) - (b.layout.order ?? 0) || - a.pluginDisplayName.localeCompare(b.pluginDisplayName) || - a.layout.label.localeCompare(b.layout.label), + compareText(a.pluginDisplayName, b.pluginDisplayName) || + compareText(a.layout.label, b.layout.label), ); } diff --git a/frontend/src/features/plugins/menus.test.ts b/frontend/src/features/plugins/menus.test.ts index 0cfcae5..1bc7fe7 100644 --- a/frontend/src/features/plugins/menus.test.ts +++ b/frontend/src/features/plugins/menus.test.ts @@ -59,6 +59,23 @@ describe("resolveTopLevelMenus", () => { const resolved = resolveTopLevelMenus(registry); expect(resolved.map((m) => m.label)).toEqual(["Z Menu", "A Menu", "B Menu"]); }); + + it("drops malformed top-level menus instead of throwing during resolution", () => { + const registry = new PluginRuntimeRegistry(); + registry.add( + stubPlugin("dev.bad", "Bad", { + ...empty(), + menus: [ + { id: "bad.menu", label: "Good", topLevel: true }, + { id: "bad.empty", label: "", topLevel: true }, + { id: undefined, label: "No id", topLevel: true }, + { id: "bad.no-label", label: undefined, topLevel: true }, + ] as unknown as PluginContributionDto["menus"], + }), + ); + + expect(resolveTopLevelMenus(registry).map((m) => m.label)).toEqual(["Good"]); + }); }); describe("resolveMenuItems", () => { @@ -137,6 +154,36 @@ describe("resolveMenuItems", () => { expect(items.map((i) => i.label)).toEqual(["A item", "Z item"]); }); + it("resolves hello-plugin's bare plugin menu target and skips malformed items", () => { + const registry = new PluginRuntimeRegistry(); + registry.add( + stubPlugin("com.example.hello-plugin", "Hello Plugin", { + ...empty(), + menus: [{ id: "hello-plugin.menu", label: "Hello", topLevel: true }], + menuItems: [ + { + id: "hello-plugin.sayHello.item", + targetMenuId: "hello-plugin.menu", + label: "Say Hello", + command: "hello-plugin.sayHello", + }, + { + id: "hello-plugin.broken.item", + targetMenuId: "hello-plugin.menu", + label: undefined, + command: "hello-plugin.broken", + }, + ] as unknown as PluginContributionDto["menuItems"], + }), + ); + + const menus = resolveTopLevelMenus(registry); + expect(menus.map((m) => m.id)).toEqual(["plugin:hello-plugin.menu"]); + const items = resolveMenuItems(registry, "plugin:hello-plugin.menu", NO_CONTEXT); + expect(items.map((i) => i.label)).toEqual(["Say Hello"]); + expect(items[0].targetMenuId).toBe("plugin:hello-plugin.menu"); + }); + it("dispatches a command via the registry's runCommand", async () => { const registry = new PluginRuntimeRegistry(); let ran = false; diff --git a/frontend/src/features/plugins/menus.ts b/frontend/src/features/plugins/menus.ts index 32253dd..61b5410 100644 --- a/frontend/src/features/plugins/menus.ts +++ b/frontend/src/features/plugins/menus.ts @@ -18,6 +18,24 @@ export interface ResolvedTopLevelMenu { order: number; } +function nonEmptyString(value: unknown): string | undefined { + return typeof value === "string" && value.trim().length > 0 ? value : undefined; +} + +function finiteOrder(value: unknown): number { + return typeof value === "number" && Number.isFinite(value) ? value : 0; +} + +function compareText(a: unknown, b: unknown): number { + return String(a ?? "").localeCompare(String(b ?? "")); +} + +function targetMatches(actual: unknown, expected: MenuTargetId): actual is MenuTargetId { + if (actual === expected) return true; + if (typeof actual !== "string") return false; + return expected.startsWith("plugin:") && actual === expected.slice("plugin:".length); +} + /** * Top-level plugin menus, sorted by `order` then plugin display name then * label (carnet §7.1) — rendered between `Panneaux` and `Paramètres`. @@ -25,19 +43,26 @@ export interface ResolvedTopLevelMenu { export function resolveTopLevelMenus(registry: PluginRuntimeRegistry): ResolvedTopLevelMenu[] { return registry .topLevelMenus() - .map(({ pluginId, pluginDisplayName, menu }) => ({ - id: `plugin:${menu.id}` as const, - pluginId, - pluginDisplayName, - label: menu.label, - icon: menu.icon, - order: menu.order ?? 0, - })) + .flatMap(({ pluginId, pluginDisplayName, menu }) => { + const id = nonEmptyString(menu.id); + const label = nonEmptyString(menu.label); + if (!id || !label) return []; + return [ + { + id: `plugin:${id}` as const, + pluginId, + pluginDisplayName, + label, + icon: nonEmptyString(menu.icon), + order: finiteOrder(menu.order), + }, + ]; + }) .sort( (a, b) => a.order - b.order || - a.pluginDisplayName.localeCompare(b.pluginDisplayName) || - a.label.localeCompare(b.label), + compareText(a.pluginDisplayName, b.pluginDisplayName) || + compareText(a.label, b.label), ); } @@ -55,27 +80,33 @@ export function resolveMenuItems( ): ResolvedPluginMenuItem[] { return registry .menuItems() - .filter(({ item }) => item.targetMenuId === targetMenuId) - .map(({ pluginId, pluginDisplayName, item }) => { + .filter(({ item }) => targetMatches(item.targetMenuId, targetMenuId)) + .flatMap(({ pluginId, pluginDisplayName, item }) => { + const id = nonEmptyString(item.id); + const label = nonEmptyString(item.label); + const command = nonEmptyString(item.command); + if (!id || !label || !command) return []; const result = evaluateWhen(item.when, whenCtx); - return { - id: item.id, - pluginId, - pluginDisplayName, - targetMenuId: item.targetMenuId, - label: item.label, - command: item.command, - enabled: result.ok ? result.value : false, - disabledReason: result.ok ? undefined : result.reason, - groupLabel: pluginDisplayName, - order: item.order ?? 0, - iconUrl: item.icon, - } satisfies ResolvedPluginMenuItem; + return [ + { + id, + pluginId, + pluginDisplayName: nonEmptyString(pluginDisplayName) ?? pluginId, + targetMenuId, + label, + command, + enabled: result.ok ? result.value : false, + disabledReason: result.ok ? undefined : result.reason, + groupLabel: nonEmptyString(pluginDisplayName) ?? pluginId, + order: finiteOrder(item.order), + iconUrl: nonEmptyString(item.icon), + } satisfies ResolvedPluginMenuItem, + ]; }) .sort( (a, b) => a.order - b.order || - a.pluginDisplayName.localeCompare(b.pluginDisplayName) || - a.label.localeCompare(b.label), + compareText(a.pluginDisplayName, b.pluginDisplayName) || + compareText(a.label, b.label), ); } diff --git a/frontend/src/plugins/runtime/loader.test.ts b/frontend/src/plugins/runtime/loader.test.ts index db1dae0..9a221e3 100644 --- a/frontend/src/plugins/runtime/loader.test.ts +++ b/frontend/src/plugins/runtime/loader.test.ts @@ -158,6 +158,43 @@ describe("loadPlugins", () => { expect(registry.get("com.example.hello-plugin")).toBeUndefined(); }); + it("loads the hello-plugin contribution shape with omitted optional arrays", async () => { + const bundle = dataUrl(` + export function activate(ctx) { + ctx.commands.registerCommand("hello-plugin.sayHello", () => { + globalThis.__helloArchiveCommandRan = true; + }); + } + `); + const { registry, failures } = await loadPlugins( + [ + entry({ + id: "com.example.hello-plugin", + displayName: "Hello Plugin", + bundleUrl: bundle, + contributes: { + menus: [{ id: "hello-plugin.menu", label: "Hello", topLevel: true }], + menuItems: [ + { + id: "hello-plugin.sayHello.item", + targetMenuId: "hello-plugin.menu", + label: "Say Hello", + command: "hello-plugin.sayHello", + }, + ], + } as unknown as PluginContributionDto, + }), + ], + gateways, + ); + + expect(failures).toEqual([]); + expect(registry.get("com.example.hello-plugin")?.contributes.layouts).toEqual([]); + expect(registry.get("com.example.hello-plugin")?.contributes.mcpServers).toEqual([]); + await registry.runCommand("com.example.hello-plugin", "hello-plugin.sayHello"); + expect((globalThis as Record).__helloArchiveCommandRan).toBe(true); + }); + it("calls dispose() on removal (best-effort)", async () => { const bundle = dataUrl(` export function activate(ctx) { diff --git a/frontend/src/plugins/runtime/loader.ts b/frontend/src/plugins/runtime/loader.ts index 48f8adb..6c7a9ca 100644 --- a/frontend/src/plugins/runtime/loader.ts +++ b/frontend/src/plugins/runtime/loader.ts @@ -15,7 +15,7 @@ * collected, never thrown past `loadPlugins`. */ -import type { PluginRuntimePlugin } from "@/domain"; +import type { PluginContributionDto, PluginRuntimePlugin } from "@/domain"; import { PluginCommandRegistry, PluginLayoutRegistry, @@ -101,6 +101,20 @@ function createCommandContext(commands: PluginCommandRegistry): PluginCommandCon }; } +function arrayOrEmpty(value: unknown): T[] { + return Array.isArray(value) ? (value as T[]) : []; +} + +function normalizeContributes(entry: PluginRuntimePlugin): PluginContributionDto { + const contributes = entry.contributes as Partial | null | undefined; + return { + menus: arrayOrEmpty(contributes?.menus), + menuItems: arrayOrEmpty(contributes?.menuItems), + layouts: arrayOrEmpty(contributes?.layouts), + mcpServers: arrayOrEmpty(contributes?.mcpServers), + }; +} + async function disposeAll(disposables: Disposable[], activation?: void | PluginActivation): Promise { try { await activation?.dispose?.(); @@ -135,8 +149,9 @@ async function loadOne( }; } - const declaredCommandIds = new Set(entry.contributes.menuItems.map((item) => item.command)); - const declaredLayoutTypes = new Set(entry.contributes.layouts.map((layout) => layout.type)); + const contributes = normalizeContributes(entry); + const declaredCommandIds = new Set(contributes.menuItems.map((item) => item.command)); + const declaredLayoutTypes = new Set(contributes.layouts.map((layout) => layout.type)); const commands = new PluginCommandRegistry(entry.id, declaredCommandIds); const layouts = new PluginLayoutRegistry(entry.id, declaredLayoutTypes); const menu = new PluginMenuRegistry(entry.id); @@ -159,7 +174,7 @@ async function loadOne( const plugin: LoadedPlugin = { pluginId: entry.id, displayName: entry.displayName, - contributes: entry.contributes, + contributes, commands, layouts, menu, diff --git a/frontend/src/plugins/runtime/registry.ts b/frontend/src/plugins/runtime/registry.ts index 3d444ae..a1afadf 100644 --- a/frontend/src/plugins/runtime/registry.ts +++ b/frontend/src/plugins/runtime/registry.ts @@ -146,6 +146,10 @@ export interface LoadedPlugin { dispose(): Promise; } +function arrayOrEmpty(value: unknown): T[] { + return Array.isArray(value) ? (value as T[]) : []; +} + /** * Aggregate, session-scoped registry every loaded plugin's contributions land * in. `PluginRuntimeRegistry` itself never imports bundles (see `loader.ts`); @@ -180,7 +184,7 @@ export class PluginRuntimeRegistry { /** All top-level menu contributions across every loaded plugin. */ topLevelMenus(): Array<{ pluginId: string; pluginDisplayName: string; menu: PluginTopLevelMenuContribution }> { return this.list().flatMap((p) => - p.contributes.menus.map((menu) => ({ + arrayOrEmpty(p.contributes.menus).map((menu) => ({ pluginId: p.pluginId, pluginDisplayName: p.displayName, menu, @@ -191,7 +195,7 @@ export class PluginRuntimeRegistry { /** All menu-item contributions across every loaded plugin. */ menuItems(): Array<{ pluginId: string; pluginDisplayName: string; item: PluginMenuItemContribution }> { return this.list().flatMap((p) => - p.contributes.menuItems.map((item) => ({ + arrayOrEmpty(p.contributes.menuItems).map((item) => ({ pluginId: p.pluginId, pluginDisplayName: p.displayName, item, @@ -214,7 +218,7 @@ export class PluginRuntimeRegistry { layout: PluginLayoutContribution; }> { return this.list().flatMap((p) => - p.contributes.layouts.map((layout) => ({ + arrayOrEmpty(p.contributes.layouts).map((layout) => ({ pluginId: p.pluginId, pluginDisplayName: p.displayName, layout,