diff --git a/src/apps/officer-web/App.tsx b/src/apps/officer-web/App.tsx index aecdbe56..27e09279 100644 --- a/src/apps/officer-web/App.tsx +++ b/src/apps/officer-web/App.tsx @@ -91,13 +91,15 @@ export function App() { {/* Installed plugins. Core routes above stay hand-written; everything below is generated from what is installed, because a bundler cannot follow a runtime import specifier. The wildcard hands the whole subtree to the plugin's own router, which react-router nests natively. */} + {/* One pair per SCREEN, not per plugin — `screenId` is the key, because a plugin with two + screens (caldav: Calendar and Contacts) contributes two entries sharing one `appName`. */} {installedPlugins.flatMap((plugin) => [ - } />, + } />, // The section pair, exactly as the core screens do it (`/soulseek/:section`): the plugin's // panels read `useParams` themselves, so which section is open is the URL rather than state // passed between them. } />, diff --git a/src/apps/officer-web/Screens/Dashboard/PluginScreen.tsx b/src/apps/officer-web/Screens/Dashboard/PluginScreen.tsx index 30074852..a59038ec 100644 --- a/src/apps/officer-web/Screens/Dashboard/PluginScreen.tsx +++ b/src/apps/officer-web/Screens/Dashboard/PluginScreen.tsx @@ -18,11 +18,18 @@ import { useAuthoredLayout } from 'state/useAuthoredLayout'; // rearranges — and `appTypes.allowed` pins it to that plugin's own panels, so a persisted layout naming // something else falls back rather than rendering another plugin's panel inside this one. export function PluginScreen({ - appName, + screenId, panels, layout, }: { - appName: string; + /** + * This SCREEN's id, not the plugin's — `caldav/calendar`, not `caldav`. + * + * A plugin with two screens has two of these, and they must differ: the id is both the layout + * persistence key and the owner of the panel allow-list below, so sharing one would have Calendar and + * Contacts persisting over each other's sizes and each accepting the other's panels. + */ + screenId: string; panels: AppRegistryMeta[]; layout: LayoutNode; }) { @@ -32,7 +39,7 @@ export function PluginScreen({ // `layout` on every render, while only the sizes are the user's and persisted. That is what lets a plugin // ship a new layout and have it reach people who have already opened the screen — and it is what stops // the `appTypes` repair below from being written back as though the user had chosen it. See the hook. - const workspace = useAuthoredLayout(`screens/plugin/${appName}`, layout); + const workspace = useAuthoredLayout(`screens/plugin/${screenId}`, layout); const allowed = panels.map((panel) => panel.key); return ( diff --git a/src/servers/plugins/discover.test.ts b/src/servers/plugins/discover.test.ts index 4f4aeab5..14daaa24 100644 --- a/src/servers/plugins/discover.test.ts +++ b/src/servers/plugins/discover.test.ts @@ -41,6 +41,50 @@ plant('broken', { 'manifest.ts': 'export const manifest = { publisher: 1 };' }); // panel, enforced here rather than by review — a `web/` directory without both files is refused. plant('halfweb', { 'manifest.ts': MANIFEST(), 'web/panels.ts': 'export const appRegistryMetas = [];' }); plant('nomanifest', { 'api/router.ts': 'export const router = {};' }); + +// ── Multi-screen plugins ── +// +// One plugin, several frontend routes: caldav is one sidecar and one permission behind Calendar and +// Contacts. The directory declares that a screen EXISTS; the manifest says what to call it and which +// glyph to draw, because a directory listing cannot say either. +const SCREEN_FILES = (segment: string) => ({ + [`web/${segment}/panels.ts`]: 'export const appRegistryMetas = [];', + [`web/${segment}/layout.ts`]: 'export const defaultLayout = {};', +}); +plant('twoscreens', { + 'manifest.ts': MANIFEST( + `screens: [{ segment: 'calendar', label: 'Calendar', icon: 'Calendar' }, { segment: 'contacts', label: 'Contacts' }],`, + ), + ...SCREEN_FILES('calendar'), + ...SCREEN_FILES('contacts'), +}); +// Declared with nothing behind it: a route that would render nothing. +plant('ghostscreen', { + 'manifest.ts': MANIFEST(`screens: [{ segment: 'calendar', label: 'Calendar' }],`), +}); +// The reverse: panels nobody declared, so a screen with no tile and no label. +plant('orphanscreen', { + 'manifest.ts': MANIFEST(`screens: [{ segment: 'calendar', label: 'Calendar' }],`), + ...SCREEN_FILES('calendar'), + ...SCREEN_FILES('contacts'), +}); +// A chosen segment is a chosen URL, and only a first-party plugin may have one — otherwise any installed +// plugin could shadow a core route, which `mountPrefix` exists to make impossible. +plant('thirdparty', { + 'manifest.ts': `export const manifest = { + publisher: 'someone-else', version: '1.0.0', platform: '>=1.0.0', + label: 'X', summary: 'x', icon: 'Network', color: '#fff', + permissions: [], screens: [{ segment: 'calendar', label: 'Calendar' }], +};`, + ...SCREEN_FILES('calendar'), +}); +// A segment is a URL segment. `../escape` would put the route somewhere other than where it reads, so it +// is refused rather than escaped. `web/` exists here on purpose, or the no-web/ check fires first and the +// segment rule is never reached. +plant('badsegment', { + 'manifest.ts': MANIFEST(`screens: [{ segment: '../escape', label: 'X' }],`), + 'web/.keep': '', +}); plant('_scratch', { 'manifest.ts': MANIFEST() }); describe('discoverPlugins', () => { @@ -50,8 +94,11 @@ describe('discoverPlugins', () => { expect(full.api).toContain('api/router.ts'); expect(full.schema).toContain('db/schema.ts'); expect(full.sidecar).toEqual({ script: join(root, 'full/sidecar/index.ts'), runtime: 'bun' }); - expect(full.web?.panels).toContain('web/panels.ts'); - expect(full.web?.layout).toContain('web/layout.ts'); + // The flat form is one screen with no segment — every plugin but caldav. + expect(full.web?.screens).toHaveLength(1); + expect(full.web?.screens[0]!.segment).toBeNull(); + expect(full.web?.screens[0]!.panels).toContain('web/panels.ts'); + expect(full.web?.screens[0]!.layout).toContain('web/layout.ts'); }); it('a manifest alone is a valid plugin — every other part is optional', async () => { @@ -75,7 +122,15 @@ describe('discoverPlugins', () => { // ones beside it. "Broken, and here is why" is renderable; a failed boot is only greppable. it('collects broken plugins instead of throwing', async () => { const { plugins, broken } = await discoverPlugins(root); - expect(broken.map((b) => b.appName).sort()).toEqual(['broken', 'halfweb', 'nomanifest']); + expect(broken.map((b) => b.appName).sort()).toEqual([ + 'badsegment', + 'broken', + 'ghostscreen', + 'halfweb', + 'nomanifest', + 'orphanscreen', + 'thirdparty', + ]); expect(broken.find((b) => b.appName === 'nomanifest')!.error).toContain('no manifest.ts'); expect(plugins.length).toBeGreaterThan(0); }); @@ -98,3 +153,45 @@ describe('discoverPlugins', () => { expect(await discoverPlugins(join(root, 'does-not-exist'))).toEqual({ plugins: [], broken: [] }); }); }); + +describe('a plugin with more than one screen', () => { + it('reads one screen per declared segment, in manifest order', async () => { + const { plugins } = await discoverPlugins(root); + const two = plugins.find((p) => p.appName === 'twoscreens')!; + expect(two.web?.screens.map((s) => s.segment)).toEqual(['calendar', 'contacts']); + expect(two.web?.screens.map((s) => s.label)).toEqual(['Calendar', 'Contacts']); + expect(two.web?.screens[0]!.panels).toContain(join('web', 'calendar', 'panels.ts')); + }); + + it("a screen with no icon falls back to the plugin's", async () => { + const { plugins } = await discoverPlugins(root); + const two = plugins.find((p) => p.appName === 'twoscreens')!; + expect(two.web?.screens[0]!.icon).toBe('Calendar'); + expect(two.web?.screens[1]!.icon).toBe('Network'); + }); + + const errorFor = async (appName: string) => { + const { broken } = await discoverPlugins(root); + return broken.find((b) => b.appName === appName)?.error ?? ''; + }; + + it('refuses a manifest that declares screens with no web/ at all', async () => { + expect(await errorFor('ghostscreen')).toContain('no web/ directory'); + }); + + it('refuses panels nobody declared — it would have no dock tile', async () => { + const orphan = await errorFor('orphanscreen'); + expect(orphan).toContain('web/contacts/'); + expect(orphan).toContain('no dock tile'); + }); + + it('refuses a THIRD-PARTY plugin choosing its own URLs', async () => { + // The whole reason a chosen segment is safe: `mountPrefix` confines third parties to /p//, + // and a segment steps out of it. Without this check, any installed plugin could shadow a core route. + expect(await errorFor('thirdparty')).toContain('first-party'); + }); + + it('refuses a segment that is not a URL segment', async () => { + expect(await errorFor('badsegment')).toContain('lowercase letters'); + }); +}); diff --git a/src/servers/plugins/discover.ts b/src/servers/plugins/discover.ts index 5ae8f7d3..04dd7207 100644 --- a/src/servers/plugins/discover.ts +++ b/src/servers/plugins/discover.ts @@ -1,7 +1,13 @@ import { existsSync, readdirSync, statSync } from 'node:fs'; import { join } from 'node:path'; import { PLATFORM_DIR } from '../data-path'; -import { manifestProblems, type DiscoveredPlugin, type PluginManifest } from './manifest'; +import { + manifestProblems, + FIRST_PARTY_PUBLISHER, + type DiscoveredPlugin, + type DiscoveredScreen, + type PluginManifest, +} from './manifest'; import { importFresh } from './import-fresh'; // Finding plugins on disk. @@ -54,22 +60,92 @@ function findSidecar(dir: string): DiscoveredPlugin['sidecar'] { * workspace with at least one panel" is enforced: there is no way to ship a frontend that is not a * workspace, because the plugin never renders the screen — it hands over panels and an arrangement. */ -function findWeb(dir: string, appName: string): { web: DiscoveredPlugin['web']; error?: string } { +function findWeb( + dir: string, + appName: string, + manifest: PluginManifest | null, +): { web: DiscoveredPlugin['web']; error?: string } { const webDir = join(dir, 'web'); - if (!existsSync(webDir)) return { web: null }; + const declared = manifest?.screens ?? []; - const panels = join(webDir, 'panels.ts'); - const layout = join(webDir, 'layout.ts'); - const missing = [!existsSync(panels) && 'web/panels.ts', !existsSync(layout) && 'web/layout.ts'].filter(Boolean); - if (missing.length) { + if (!existsSync(webDir)) { + // No `web/` and no claim to one is the ordinary backend-only plugin. No `web/` while DECLARING + // screens is a manifest describing a frontend that is not there — every route it names would 404, + // and the dock would draw tiles pointing at them. + if (!declared.length) return { web: null }; + return { + web: null, + error: `${appName}: declares ${declared.length} screen(s) but has no web/ directory.`, + }; + } + + // The flat form: one screen at the mount prefix. Every plugin but caldav, and the reason adding + // multi-screen support moved none of them. + if (!declared.length) { + const panels = join(webDir, 'panels.ts'); + const layout = join(webDir, 'layout.ts'); + const missing = [!existsSync(panels) && 'web/panels.ts', !existsSync(layout) && 'web/layout.ts'].filter(Boolean); + if (missing.length) { + return { + web: null, + error: + `${appName}: has a web/ directory but is missing ${missing.join(' and ')}. ` + + `Every plugin route renders a Workspace: contribute panels and a layout, not a screen.`, + }; + } + return { + web: { screens: [{ segment: null, label: manifest?.label ?? appName, icon: manifest?.icon, panels, layout }] }, + }; + } + + // A named segment is a URL the plugin chose rather than one derived from its name, and `mountPrefix` + // is what stops that being a way to shadow a core route — it confines third parties to + // `/p//`, which a chosen segment would step straight out of. So this is first-party only, + // refused here rather than anywhere later: by the time a route exists it is already serving. + if (manifest && manifest.publisher !== FIRST_PARTY_PUBLISHER) { return { web: null, error: - `${appName}: has a web/ directory but is missing ${missing.join(' and ')}. ` + - `Every plugin route renders a Workspace: contribute panels and a layout, not a screen.`, + `${appName}: declares screens, which chooses its own URLs, and only a first-party plugin may. ` + + `A third-party plugin mounts under /p/${manifest.publisher}/ so it cannot shadow a core route.`, }; } - return { web: { panels, layout } }; + + const screens: DiscoveredScreen[] = []; + const problems: string[] = []; + + for (const screen of declared) { + // A segment is a URL segment. Anything else — a separator, a traversal, a space — would land the + // route somewhere other than where it reads, so it is refused rather than escaped. + if (!/^[a-z0-9][a-z0-9-]*$/.test(screen.segment)) { + problems.push(`screen segment ${JSON.stringify(screen.segment)} must be lowercase letters, digits and dashes`); + continue; + } + const panels = join(webDir, screen.segment, 'panels.ts'); + const layout = join(webDir, screen.segment, 'layout.ts'); + const missing = [ + !existsSync(panels) && `web/${screen.segment}/panels.ts`, + !existsSync(layout) && `web/${screen.segment}/layout.ts`, + ].filter(Boolean); + if (missing.length) { + problems.push(`screen "${screen.segment}" is declared but missing ${missing.join(' and ')}`); + continue; + } + screens.push({ segment: screen.segment, label: screen.label, icon: screen.icon ?? manifest?.icon, panels, layout }); + } + + // A directory nobody declared is a screen with no tile and no label — it would render at a URL with + // nothing pointing at it. Reported for the same reason the reverse is: both are the quiet kind of broken. + const undeclared = readdirSync(webDir, { withFileTypes: true }) + .filter((entry) => entry.isDirectory() && existsSync(join(webDir, entry.name, 'panels.ts'))) + .map((entry) => entry.name) + .filter((name) => !declared.some((screen) => screen.segment === name)); + for (const name of undeclared) { + problems.push(`web/${name}/ has panels but no screens entry — it would have no dock tile and no label`); + } + + if (problems.length) return { web: null, error: `${appName}: ${problems.join('; ')}.` }; + return { web: { screens } }; } const fileOrNull = (path: string): string | null => (existsSync(path) ? path : null); @@ -97,7 +173,7 @@ export async function loadPlugin(dir: string, appName: string): Promise { - const { plugin } = state; - const alias = `Plugin${i}`; - imports.push(`import { appRegistryMetas as ${alias}Panels } from '${specifier(plugin.web!.panels)}';`); - imports.push(`import { defaultLayout as ${alias}Layout } from '${specifier(plugin.web!.layout)}';`); - entries.push( - ` { appName: '${plugin.appName}', route: '${mountPrefix(plugin)}', panels: ${alias}Panels, layout: ${alias}Layout },`, - ); - }); + // One entry per SCREEN, not per plugin. Almost every plugin has exactly one and this reads the same as + // it always did; caldav has two, so it emits two routes, two layout keys and two dock tiles from one + // install. The alias counter runs over screens for that reason — two screens of one plugin must not + // share an import name. + let i = 0; + for (const { plugin } of withWeb) { + for (const screen of plugin.web!.screens) { + const alias = `Plugin${i}`; + i += 1; + imports.push(`import { appRegistryMetas as ${alias}Panels } from '${specifier(screen.panels)}';`); + imports.push(`import { defaultLayout as ${alias}Layout } from '${specifier(screen.layout)}';`); + // The screen id is the LAYOUT KEY (`screens/plugin/`) and the panel allow-list's owner, so two + // screens of one plugin must not share it — otherwise they persist over each other's sizes and each + // would accept the other's panels. + const screenId = screen.segment ? `${plugin.appName}/${screen.segment}` : plugin.appName; + const route = screen.segment ? `/${screen.segment}` : mountPrefix(plugin); + entries.push( + ` { appName: '${plugin.appName}', screenId: '${screenId}', route: '${route}', panels: ${alias}Panels, layout: ${alias}Layout },`, + ); + } + } const body = `${HEADER} import type { AppRegistryMeta, LayoutNode } from 'officerdev'; ${imports.join('\n')} export type GeneratedPlugin = { + /** The plugin this screen belongs to. Two screens of one plugin share it. */ appName: string; - /** Where it mounts, from \`mountPrefix\` — \`/offscale\` for ours, \`/p//\` for others. */ + /** + * This SCREEN's id — \`\` for a single-screen plugin, \`/\` for one of several. + * + * It is the layout persistence key and the panel allow-list's owner, so it has to be unique per screen + * rather than per plugin: sharing it would have two screens persisting over each other's sizes. + */ + screenId: string; + /** Where it mounts — \`mountPrefix\` normally, or a first-party plugin's chosen segment. */ route: string; /** The panels it contributes. At least one, or discovery refuses the plugin. */ panels: AppRegistryMeta[]; diff --git a/src/servers/plugins/manifest.ts b/src/servers/plugins/manifest.ts index 584dd941..4caeeb04 100644 --- a/src/servers/plugins/manifest.ts +++ b/src/servers/plugins/manifest.ts @@ -238,6 +238,30 @@ export type PluginManifest = { permissions: PluginPermission[]; + /** + * The frontend screens this plugin contributes, when it contributes more than one. + * + * Omit it and the plugin has exactly one screen at its mount prefix, labelled `label` — which is every + * plugin but caldav, and the reason nothing else had to change when this arrived. + * + * ── Why the label is HERE and the existence is on disk ── + * + * Presence is the declaration everywhere else in this file, and it still is: `web//panels.ts` + * is what makes a screen exist. What a directory listing cannot say is what to CALL it and which glyph + * to draw, and those are exactly what this file is for — a human choice, not a structural fact. + * + * The two are cross-checked at discovery and a mismatch is REFUSED, not reconciled: a segment here with + * no directory, or a directory with no entry here, names a screen that would render nothing or a screen + * with no tile. Both are the quiet kind of broken. + * + * ── The segment is a URL a first-party plugin chooses ── + * + * `/calendar` and `/contacts`, not `/caldav/calendar`. That is only sound because `mountPrefix` confines + * third parties to `/p//` — so a third-party plugin declaring `screens` is refused. Without + * that check this field would be a way for any installed plugin to shadow a core route. + */ + screens?: { segment: string; label: string; icon?: string }[]; + /** * Paths under this plugin's own mount that are served ABOVE the account gate. * @@ -478,7 +502,36 @@ export type DiscoveredPlugin = { * Both files are required when `web/` exists at all. Missing either is a broken plugin, not a plugin * with a partial frontend. */ - web: { panels: string; layout: string } | null; + web: { screens: DiscoveredScreen[] } | null; +}; + +/** + * One frontend screen: a route, the panels on it, and how they are arranged. + * + * ── Why a plugin can have more than one ── + * + * A plugin was one screen because the two things it declares — a mount prefix and a frontend — happened to + * be the same string. Caldav is where that stops holding: one sidecar, one permission, one install, and + * two screens that have nothing to do with each other on screen. Calendar and Contacts are not sections of + * one page and forcing them into one would be a worse product than the core screens they replace. + * + * ── segment ── + * + * `null` for the ordinary plugin, which means "mount at the prefix" and is every plugin but this one. A + * named segment is a first-party plugin choosing its own URL — `/calendar`, not `/caldav/calendar` — which + * is only safe because `mountPrefix` still confines third parties to `/p//`. A third-party + * plugin naming a segment is refused at discovery, and that check is what keeps this from becoming a way + * to shadow a core route. + */ +export type DiscoveredScreen = { + /** Directory name under `web/`, or null for the flat single-screen form. */ + segment: string | null; + /** Dock tile label. The manifest's own `label` for the flat form. */ + label: string; + /** Lucide icon NAME for this screen's tile, falling back to the plugin's. */ + icon?: string; + panels: string; + layout: string; }; /** diff --git a/src/servers/plugins/mount.ts b/src/servers/plugins/mount.ts index 649d0d51..8dedff1f 100644 --- a/src/servers/plugins/mount.ts +++ b/src/servers/plugins/mount.ts @@ -289,23 +289,37 @@ export async function pluginDockManifests(): Promise< }> > { const { states } = await snapshotPlugins(); + // One tile per SCREEN. A plugin with two screens gets two tiles behind ONE permission — caldav is + // Calendar and Contacts, which are two places to go and one thing to be allowed to reach. Filtering + // stays per-plugin for that reason: the permission is the plugin's, and a screen is not a smaller + // grant, it is another door into the same room. return states .filter((s) => s.install?.enabled && s.plugin.web) - .map(({ plugin }) => { - const prefix = mountPrefix(plugin); - return { - sidecarId: plugin.appName, - permission: plugin.manifest.permissions[0]?.key ?? null, - name: plugin.manifest.label, - // A shipped `assets/icon.png` wins; the lucide NAME is the fallback for a plugin with no - // artwork of its own. The dock renders `image` as an and `icon` through `resolveIcon`, - // and it already preferred the image — this is the first thing to give it one. - ...(plugin.icon ? { image: iconUrl(plugin.appName) } : { icon: plugin.manifest.icon }), - // Artwork defaults to `bare`, a glyph to `badge`; the manifest overrides either way. - tile: plugin.manifest.tile ?? (plugin.icon ? 'bare' : 'badge'), - color: plugin.manifest.color, - rootRoute: prefix, - routes: [prefix], - }; - }); + .flatMap(({ plugin }) => + plugin.web!.screens.map((screen) => { + const route = screen.segment ? `/${screen.segment}` : mountPrefix(plugin); + return { + sidecarId: plugin.appName, + permission: plugin.manifest.permissions[0]?.key ?? null, + name: screen.label, + // A shipped `assets/icon.png` wins; the lucide NAME is the fallback for a plugin with no + // artwork of its own. The dock renders `image` as an and `icon` through `resolveIcon`, + // and it already preferred the image — this is the first thing to give it one. + // + // A plugin with several screens is the one case where the shared artwork is WRONG: two + // identical tiles side by side say nothing about which is which. So a screen naming its own + // glyph overrides the image, and one that does not falls back to the plugin's. + ...(screen.icon && plugin.web!.screens.length > 1 + ? { icon: screen.icon } + : plugin.icon + ? { image: iconUrl(plugin.appName) } + : { icon: screen.icon ?? plugin.manifest.icon }), + // Artwork defaults to `bare`, a glyph to `badge`; the manifest overrides either way. + tile: plugin.manifest.tile ?? (plugin.icon && plugin.web!.screens.length === 1 ? 'bare' : 'badge'), + color: plugin.manifest.color, + rootRoute: route, + routes: [route], + }; + }), + ); }