merge fix/hello-plugin-black-window dans develop (#hello-plugin: isole les contributions plugin en erreur et durcit menus.ts)

QA: verdict VERT (suite automatisée), réserve manuelle : vérification desktop (chargement réel de l'archive dans l'app packagée) non exécutée.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
2026-07-30 09:08:25 +02:00
9 changed files with 316 additions and 58 deletions

View File

@ -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();
}
});
});

View File

@ -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);
if (availability !== "available") {
const providerDisplayName =
registry.get(cell.pluginId)?.displayName ??
installedPlugins?.find((p) => p.id === cell.pluginId)?.displayName;
if (availability !== "available") {
return (
<PluginLayoutFallback
cell={cell}
@ -53,7 +87,20 @@ export function PluginLayoutCellView({
}
const Component = registry.layoutComponent(cell.pluginId, cell.layoutType)!;
const fallback = (
<PluginLayoutFallback
cell={cell}
availability="incompatible"
providerDisplayName={providerDisplayName}
onOpenPlugins={onOpenPlugins}
onChooseAnotherLayout={onChooseAnotherLayout}
/>
);
return (
<PluginLayoutErrorBoundary
resetKey={`${cell.pluginId}:${cell.layoutType}:${cell.id}`}
fallback={fallback}
>
<Component
projectId={projectId}
nodeId={cell.id}
@ -69,5 +116,6 @@ export function PluginLayoutCellView({
system: gateways.system,
}}
/>
</PluginLayoutErrorBoundary>
);
}

View File

@ -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");
});
});

View File

@ -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),
);
}

View File

@ -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;

View File

@ -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,
.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: menu.label,
icon: menu.icon,
order: menu.order ?? 0,
}))
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,
return [
{
id,
pluginId,
pluginDisplayName,
targetMenuId: item.targetMenuId,
label: item.label,
command: item.command,
pluginDisplayName: nonEmptyString(pluginDisplayName) ?? pluginId,
targetMenuId,
label,
command,
enabled: result.ok ? result.value : false,
disabledReason: result.ok ? undefined : result.reason,
groupLabel: pluginDisplayName,
order: item.order ?? 0,
iconUrl: item.icon,
} satisfies ResolvedPluginMenuItem;
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),
);
}

View File

@ -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<string, unknown>).__helloArchiveCommandRan).toBe(true);
});
it("calls dispose() on removal (best-effort)", async () => {
const bundle = dataUrl(`
export function activate(ctx) {

View File

@ -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<T>(value: unknown): T[] {
return Array.isArray(value) ? (value as T[]) : [];
}
function normalizeContributes(entry: PluginRuntimePlugin): PluginContributionDto {
const contributes = entry.contributes as Partial<PluginContributionDto> | 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<void> {
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,

View File

@ -146,6 +146,10 @@ export interface LoadedPlugin {
dispose(): Promise<void>;
}
function arrayOrEmpty<T>(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<PluginTopLevelMenuContribution>(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<PluginMenuItemContribution>(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<PluginLayoutContribution>(p.contributes.layouts).map((layout) => ({
pluginId: p.pluginId,
pluginDisplayName: p.displayName,
layout,