From 29170dd9220bfb6be4e6fd4087362c3c6ccc1dbc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Padez?= Date: Sat, 15 Aug 2026 18:08:05 +0000 Subject: [PATCH] installing a plugin pins its dock tile, with no reload MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Installing Music left the dock unchanged. The tile existed server-side the moment the plugin was enabled, and the only way to see it was Profile → Dock, by hand, after a page refresh. Installing something is already the decision to use it; saying so twice is the bug. Three separate causes, which is why it looked like one stubborn one: 1. `dock_configs.paths` is a COMPLETE ordered list, not a diff — useDock renders the saved list rather than everything available, so anything not named in it is invisible. Nothing in the plugin system had ever written to that table. installPlugin now appends the new route, for the caller (threaded through the route) rather than a guessed owner id. Only for a user who HAS a row. A user without one is left alone deliberately: there is nothing to append to, and writing one would freeze today's defaults into their account permanently. That case is (2). 2. The fallback for a user who never customised was DEFAULT_DOCK_PATHS, which is core-only — so a plugin tile was hidden there too. `defaultDockPaths(plugins)` is now that list plus a tile per installed plugin, and the dock and its settings screen both call it so they cannot disagree about what "default" means. 3. useDock holds ['DOCK'] at staleTime: Infinity and the plugin verbs invalidated only ['plugins'] and ['self-permissions']. So even once the server wrote the path, nothing asked again — that is the part that made a reload necessary. Also fixes a guard I broke reaching for this: typing VERBS as Record widens keyof to `string`, which makes `isVerb` prove nothing and lets an unknown verb index the map. `satisfies` instead, and there is now a check that Verb is still the four-member union. Uninstall deliberately does not remove the path. A stale path with no item behind it is dropped by useDock, and keeping it means disable/enable and reinstall put the tile back where the user had it rather than at the end. tsgo clean, frontend builds. 787 pass / 7 fail — 6 identical to before; the 7th is plugins/music/cliamp, which install pulled into test discovery and which the manifest documents as parked. Co-Authored-By: Claude Opus 5 --- plugins/music | 1 + .../Dashboard/Layout/DashboardLayout.tsx | 7 +++- .../Screens/Dashboard/Layout/Dock.tsx | 16 ++++++++ .../Settings/ProfileSettings/DockSettings.tsx | 7 +++- src/servers/api/plugins/router.ts | 22 ++++++---- src/servers/plugins/install.ts | 41 +++++++++++++++++-- .../officerdev/src/apps/Plugins/usePlugins.ts | 10 ++++- 7 files changed, 88 insertions(+), 16 deletions(-) create mode 160000 plugins/music diff --git a/plugins/music b/plugins/music new file mode 160000 index 00000000..6e07da7a --- /dev/null +++ b/plugins/music @@ -0,0 +1 @@ +Subproject commit 6e07da7a3650786e5ef729ee6ae7ff2b6a37862a diff --git a/src/apps/officer-web/Screens/Dashboard/Layout/DashboardLayout.tsx b/src/apps/officer-web/Screens/Dashboard/Layout/DashboardLayout.tsx index 0c1727cc..82908fd8 100644 --- a/src/apps/officer-web/Screens/Dashboard/Layout/DashboardLayout.tsx +++ b/src/apps/officer-web/Screens/Dashboard/Layout/DashboardLayout.tsx @@ -6,7 +6,7 @@ import { ErrorBoundary } from '@/components/ErrorBoundary'; import { ScreenErrorFallback } from './ScreenErrorFallback'; import { Background } from './Background'; import { Header } from './Header'; -import { Dock, CORE_DOCK_ITEMS, dockItemsFromPlugins, DEFAULT_DOCK_PATHS } from './Dock'; +import { Dock, CORE_DOCK_ITEMS, dockItemsFromPlugins, defaultDockPaths } from './Dock'; import { useIsTouch } from './useIsTouch'; import { usePageTitleSync } from '@/state/usePageTitle'; import { RouteGate } from './RouteGate'; @@ -26,7 +26,10 @@ export function DashboardLayout({ children }: DashboardLayoutProps) { () => [...CORE_DOCK_ITEMS, ...dockItemsFromPlugins(plugins)].filter((item) => canVisit(item.to)), [canVisit, plugins], ); - const { items: visibleItems } = useDock(permitted, DEFAULT_DOCK_PATHS); + const { items: visibleItems } = useDock( + permitted, + useMemo(() => defaultDockPaths(plugins), [plugins]), + ); const isTouch = useIsTouch(); const { pathname } = useLocation(); usePageTitleSync(); diff --git a/src/apps/officer-web/Screens/Dashboard/Layout/Dock.tsx b/src/apps/officer-web/Screens/Dashboard/Layout/Dock.tsx index b8ddd249..d953e48d 100644 --- a/src/apps/officer-web/Screens/Dashboard/Layout/Dock.tsx +++ b/src/apps/officer-web/Screens/Dashboard/Layout/Dock.tsx @@ -236,3 +236,19 @@ export function dockItemsFromPlugins(plugins: PluginManifest[]): DockItem[] { * looking subtly wrong on a fresh install for no stated reason. */ export const DEFAULT_DOCK_PATHS = ['/', '/files', '/terminal', '/dashboards', '/chat']; + +/** + * The same defaults, plus a tile for every installed plugin. + * + * A plugin's tile is hidden by DEFAULT_DOCK_PATHS alone, because that list is core-only and `useDock` + * renders the saved list rather than everything available. For someone who has never touched their dock + * that reads as the install silently not working — they installed Music and the dock looks identical. + * + * Installing IS the choice, so the default follows it. This covers the never-customised user; someone + * with a saved dock has a row instead, and the installer appends to that (see `plugins/install.ts`). + * Both call sites use this so the dock and its settings screen cannot disagree about what "default" means. + */ +export const defaultDockPaths = (plugins: PluginManifest[]): string[] => [ + ...DEFAULT_DOCK_PATHS, + ...dockItemsFromPlugins(plugins).map((item) => item.to), +]; diff --git a/src/apps/officer-web/Screens/Dashboard/Settings/ProfileSettings/DockSettings.tsx b/src/apps/officer-web/Screens/Dashboard/Settings/ProfileSettings/DockSettings.tsx index 56538248..e3c54614 100644 --- a/src/apps/officer-web/Screens/Dashboard/Settings/ProfileSettings/DockSettings.tsx +++ b/src/apps/officer-web/Screens/Dashboard/Settings/ProfileSettings/DockSettings.tsx @@ -3,7 +3,7 @@ import { X, Plus, RotateCcw } from 'lucide-react'; import { Button } from '@/components/ui/button'; import { useDock } from 'officerdev'; import { usePermissions } from 'hooks/usePermissions'; -import { CORE_DOCK_ITEMS, dockItemsFromPlugins, DEFAULT_DOCK_PATHS } from '@/Screens/Dashboard/Layout/Dock'; +import { CORE_DOCK_ITEMS, dockItemsFromPlugins, defaultDockPaths } from '@/Screens/Dashboard/Layout/Dock'; type DockPillProps = { label: string; @@ -108,7 +108,10 @@ export const DockSettings = () => { // pin a tile that cannot appear, which reads as the setting being broken. const { plugins } = usePermissions(); const allDockItems = useMemo(() => [...CORE_DOCK_ITEMS, ...dockItemsFromPlugins(plugins)], [plugins]); - const { items, allItems, setItems, reset } = useDock(allDockItems, DEFAULT_DOCK_PATHS); + const { items, allItems, setItems, reset } = useDock( + allDockItems, + useMemo(() => defaultDockPaths(plugins), [plugins]), + ); const [dropTarget, setDropTarget] = useState<{ path: string; side: 'left' | 'right' } | null>(null); const visiblePaths = new Set(items.map((i) => i.to)); diff --git a/src/servers/api/plugins/router.ts b/src/servers/api/plugins/router.ts index 0363d29b..1cf421e6 100644 --- a/src/servers/api/plugins/router.ts +++ b/src/servers/api/plugins/router.ts @@ -153,7 +153,7 @@ pluginsRouter.get('/', async (ctx) => { * the one worth reading. See `plugins/install.ts` for why the order inside each is what it is. */ pluginsRouter.post('/:appName/install', async (ctx) => { - const result = await installPlugin(ctx.req.param('appName')); + const result = await installPlugin(ctx.req.param('appName'), undefined, ctx.get('user').id); return ctx.json(result, result.ok ? 200 : 400); }); @@ -183,12 +183,18 @@ pluginsRouter.post('/:appName/disable', async (ctx) => { // routes are owner-only. The client reads the body and parses frames itself, which is exactly what // `useCompanionLogStream` already does for the headscale container logs. +// `userId` is the caller, threaded through so install can pin the new tile to THEIR dock rather than +// guessing at the owner. Only install uses it; the others take it and ignore it so the map stays one shape. +type VerbFn = (appName: string, onStep: (s: string) => Promise, userId: number) => Promise; + const VERBS = { - install: (appName: string, onStep: (s: string) => Promise) => installPlugin(appName, onStep), - uninstall: (appName: string, onStep: (s: string) => Promise) => uninstallPlugin(appName, onStep), - enable: (appName: string, onStep: (s: string) => Promise) => setPluginRunning(appName, true, onStep), - disable: (appName: string, onStep: (s: string) => Promise) => setPluginRunning(appName, false, onStep), -} as const; + install: (appName, onStep, userId) => installPlugin(appName, onStep, userId), + uninstall: (appName, onStep) => uninstallPlugin(appName, onStep), + enable: (appName, onStep) => setPluginRunning(appName, true, onStep), + disable: (appName, onStep) => setPluginRunning(appName, false, onStep), + // `satisfies` rather than an annotation: an annotation would widen the keys to `string`, which turns + // `isVerb` into a guard that proves nothing and lets an unknown verb index the map. +} satisfies Record; type Verb = keyof typeof VERBS; const isVerb = (v: string): v is Verb => v in VERBS; @@ -197,6 +203,8 @@ pluginsRouter.post('/:appName/:verb/stream', async (ctx) => { const appName = ctx.req.param('appName'); const verb = ctx.req.param('verb'); if (!isVerb(verb)) throw errors.NOT_FOUND(`Unknown action: ${verb}`); + // Read before the stream opens: once we are inside ReadableStream.start the request context is gone. + const userId = ctx.get('user').id; const encoder = new TextEncoder(); const frame = (event: string, data: unknown) => encoder.encode(`event: ${event}\ndata: ${JSON.stringify(data)}\n\n`); @@ -215,7 +223,7 @@ pluginsRouter.post('/:appName/:verb/stream', async (ctx) => { let result: PluginActionResult; try { - result = await VERBS[verb](appName, async (step) => send('step', { step })); + result = await VERBS[verb](appName, async (step) => send('step', { step }), userId); } catch (err) { result = { ok: false, appName, steps: [], error: err instanceof Error ? err.message : String(err) }; } diff --git a/src/servers/plugins/install.ts b/src/servers/plugins/install.ts index c49f62c3..68717fe9 100644 --- a/src/servers/plugins/install.ts +++ b/src/servers/plugins/install.ts @@ -1,4 +1,4 @@ -import { recordPluginInstall, removePluginInstall, setPluginEnabled } from 'officerdb'; +import { getDockPaths, recordPluginInstall, removePluginInstall, setDockPaths, setPluginEnabled } from 'officerdb'; import { PLATFORM_DIR } from '../data-path'; import { deleteProcess, processStatus, startProcess, stopProcess } from '../app-store/pm2'; import { addPluginToEcosystem, pluginProcessName, removePluginFromEcosystem } from './ecosystem'; @@ -8,7 +8,7 @@ import { publishAssets, unpublishAssets } from '../app-store/assets'; import { refreshPluginMounts, snapshotPlugins } from './mount'; import { generatePluginSchemas, pushSchema } from './schema'; import { installDependencies, manualInstallHint, reportDependencies, stillMissing } from './os-deps'; -import type { DiscoveredPlugin } from './manifest'; +import { mountPrefix, type DiscoveredPlugin } from './manifest'; // The install runner: the four verbs, each as a short ordered list of effects. // @@ -86,6 +86,34 @@ async function findPlugin(appName: string): Promise { return states.find((s) => s.plugin.appName === appName)?.plugin ?? null; } +/** + * Put the new plugin's tile on the installing user's dock. + * + * Only for a user who has SAVED a dock. That list is a complete, ordered replacement — `useDock` renders + * it rather than everything available — so a plugin installed after they arranged it is invisible until + * they go to Profile → Dock and add it by hand. Installing something is already the decision to use it; + * making them say so twice is the bug. + * + * A user with no row is deliberately left alone. There is nothing to append to, and writing one here + * would freeze today's defaults into their account forever — the frontend covers that case instead, by + * including plugin tiles in the default itself (`defaultDockPaths` in Dock.tsx). + * + * Appended, never inserted, so it lands at the end where a new thing belongs and their ordering survives. + * + * Best-effort by design: the plugin is installed and working at this point, and failing the whole install + * over a dock preference would be the tail wagging the dog. + */ +async function addToDock(userId: number, route: string): Promise { + try { + const saved = await getDockPaths(userId); + if (!saved || saved.includes(route)) return false; + await setDockPaths(userId, [...saved, route]); + return true; + } catch { + return false; + } +} + /** * Install, or upgrade one already installed. * @@ -95,7 +123,7 @@ async function findPlugin(appName: string): Promise { * `enabled` is deliberately untouched on the upgrade path — re-installing something the owner had * switched off must not switch it back on. */ -export async function installPlugin(appName: string, onStep?: OnStep): Promise { +export async function installPlugin(appName: string, onStep?: OnStep, forUserId?: number): Promise { const steps: string[] = []; // FETCH FIRST, when the plugin is not here yet. @@ -205,6 +233,13 @@ export async function installPlugin(appName: string, onStep?: OnStep): Promise

:server`), and it diff --git a/src/workspaces/officerdev/src/apps/Plugins/usePlugins.ts b/src/workspaces/officerdev/src/apps/Plugins/usePlugins.ts index 4633d59a..7ceb0397 100644 --- a/src/workspaces/officerdev/src/apps/Plugins/usePlugins.ts +++ b/src/workspaces/officerdev/src/apps/Plugins/usePlugins.ts @@ -118,11 +118,17 @@ export function usePlugins() { client.get<{ plugins: PluginItem[]; available: AvailablePlugin[]; broken: BrokenPlugin[] }>('/plugins'), }); - // Every verb invalidates the plugin list AND self-permissions: installing a plugin can add a dock tile - // and a route the shell has to know about, so refreshing one without the other leaves the two disagreeing. + // Every verb invalidates three caches, because a dock tile is assembled from all three: the plugin list, + // the manifests the shell renders tiles from (`self-permissions`), and the user's saved dock. Refreshing + // any two leaves them disagreeing. + // + // `DOCK` is the one that is easy to miss and it is the reason an install used to need a page reload: the + // installer appends the new route to the saved dock server-side, and `useDock` holds that answer at + // `staleTime: Infinity`, so nothing short of invalidation would ever ask again. const invalidate = useCallback(() => { queryClient.invalidateQueries({ queryKey: PLUGINS_KEY }); queryClient.invalidateQueries({ queryKey: ['self-permissions'] }); + queryClient.invalidateQueries({ queryKey: ['DOCK'] }); }, [queryClient]); const [steps, setSteps] = useState([]);