diff --git a/src/servers/api/plugins/router.ts b/src/servers/api/plugins/router.ts index 30ed29aa..c53c2845 100644 --- a/src/servers/api/plugins/router.ts +++ b/src/servers/api/plugins/router.ts @@ -6,6 +6,8 @@ import { snapshotPlugins } from '../../plugins/mount'; import { manualInstallHint, reportDependencies } from '../../plugins/os-deps'; import { fetchCatalogue, fetchReadme } from '../../plugins/marketplace'; import { findOrphanTables } from '../../plugins/orphans'; +import { fetchPlugin } from '../../plugins/fetch'; +import { discoverPlugins } from '../../plugins/discover'; import { iconUrl } from '../../app-store/assets'; import { installPlugin, @@ -224,29 +226,6 @@ pluginsRouter.post('/update-all', async (ctx) => { }); }); -/* - POST /plugins/install-from-url — a plugin that is in no registry. - - The catalogue was the only way a remote plugin could arrive, so "build one and give it to a friend" - had no route at all. The first real case is a plugin that must never be listed: fofos-admin is one - website's admin panel, useful to exactly one person, and putting it in a public registry to install it - would be the wrong shape. - - ── The name comes from the URL, because the name IS the directory ── - - `appName` is taken from the directory a plugin sits in and never declared in its manifest, so the id - cannot disagree with where the code is. That decision is made for us here: the repository name becomes - the directory, and therefore the id, the route segment, the table prefix and the sidecar suffix. A - repository called something unusable as an id is refused before anything is cloned rather than after — - a half-clone under a bad name is a plugin the platform reports as broken instead of absent. - - ── Owner-only, and it is not the same owner-only as the rest of this router ── - - Every route here is owner-gated because installing a plugin mounts routes and starts a process. This - one also takes an ADDRESS from the caller, so the owner is choosing what code runs, not choosing from - a catalogue somebody curated. The URL is validated in `fetch.ts` — https, no credentials, no shell — - but validation is not review: nothing here reads what it cloned. -*/ /** * `https://host/owner/repo` or `…/repo.git` → `repo`. * @@ -255,7 +234,7 @@ pluginsRouter.post('/update-all', async (ctx) => { * prefix and the sidecar suffix. Checked before anything is cloned: a half-clone under an unusable name * reads as a BROKEN plugin rather than an absent one, which is much harder to get out of. */ -export function pluginNameFromUrl(url: string, override?: string): { appName: string } | { error: string } { +function pluginNameFromUrl(url: string, override?: string): { appName: string } | { error: string } { const derived = url .replace(/\.git$/, '') .replace(/\/+$/, '') @@ -272,7 +251,34 @@ export function pluginNameFromUrl(url: string, override?: string): { appName: st return { appName }; } -pluginsRouter.post('/install-from-url', async (ctx) => { +/* + POST /plugins/fetch-from-url — put a plugin on the machine, and stop there. + + The catalogue was the only way a remote plugin could arrive, so "build one and give it to a friend" + had no route. The first real case is one that must never be listed: fofos-admin is a single website's + admin panel. + + ── Why this FETCHES and does not install ── + + It was an install first, and that was wrong in a way only using it showed: the button lives on the + grid, the progress log lives in the detail panel, so a URL install ran to completion with nothing on + screen and the plugin simply appeared, installed. Streaming into a view that cannot render it is worse + than not streaming. + + Cloning alone is enough to make a plugin real — `/api/plugins` lists what is ON DISK with + `installed: false`, so it shows up as Available like any catalogue entry. From there the existing card, + the existing Install button and the existing streamed progress do the rest. One flow, not two. + + ── The name still comes from the URL ── + + `appName` IS the directory, so the repository name decides the route segment, the table prefix and the + process name. Checked before cloning: a half-clone under an unusable name reads as a BROKEN plugin + rather than an absent one. + + Owner-only like everything else here, and worth saying plainly — this puts somebody's code on the + machine. It does not run it, which is exactly why installing stays a separate, deliberate click. +*/ +pluginsRouter.post('/fetch-from-url', async (ctx) => { type FromUrl = { url?: string; appName?: string }; const body = await ctx.req.json().catch(() => ({}) as FromUrl); const url = body.url?.trim(); @@ -280,9 +286,30 @@ pluginsRouter.post('/install-from-url', async (ctx) => { const named = pluginNameFromUrl(url, body.appName); if ('error' in named) throw errors.BAD_REQUEST(named.error); + const { appName } = named; - const result = await installPlugin(named.appName, undefined, ctx.get('user').id, undefined, url); - return ctx.json(result, result.ok ? 200 : 400); + const fetched = await fetchPlugin(appName, undefined, url); + if (!fetched.ok) throw errors.BAD_REQUEST(fetched.error ?? `could not fetch ${appName}`); + + /* + Re-read from disk rather than trusting the clone. + + `fetchPlugin` checks that `manifest.ts` EXISTS; discovery checks that it PARSES and says what a + plugin needs to say. A directory that fails the second test still renders in the grid — as broken, + which is the honest state — but saying so here means the person hears it while they are still + looking at the field they pasted into. + */ + const { plugins, broken } = await discoverPlugins(); + const found = plugins.find((entry) => entry.appName === appName); + const bad = broken.find((entry) => entry.appName === appName); + if (!found) { + return ctx.json( + { ok: false, appName, error: bad?.error ?? `cloned "${appName}", but it does not read as a plugin` }, + 400, + ); + } + + return ctx.json({ ok: true, appName, label: found.manifest?.label ?? appName }); }); pluginsRouter.post('/:appName/install', async (ctx) => { @@ -355,77 +382,6 @@ const VERBS = { type Verb = keyof typeof VERBS; const isVerb = (v: string): v is Verb => v in VERBS; -/* - The same install, streamed — so the marketplace's "Install from Git repository" watches it happen. - - A twin of `/:appName/:verb/stream` below rather than a case inside it, because the URL install is the - one verb whose SUBJECT arrives in the body: every other verb names an installed plugin in its path, and - this one is naming something the machine has never seen. Folding it into the verb map would mean a - route whose `:appName` is ignored for exactly one value of `:verb`. - - Everything after the name is resolved is identical — same `installPlugin`, same step and log events, - same `done` frame — so the client renders it with the machinery it already has. -*/ -pluginsRouter.post('/install-from-url/stream', async (ctx) => { - type FromUrl = { url?: string; appName?: string }; - const body = await ctx.req.json().catch(() => ({}) as FromUrl); - const url = body.url?.trim(); - if (!url) throw errors.BAD_REQUEST('url is required'); - - const named = pluginNameFromUrl(url, body.appName); - if ('error' in named) throw errors.BAD_REQUEST(named.error); - const { appName } = named; - - // Read before the stream opens: once 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`); - - const stream = new ReadableStream({ - async start(controller) { - const send = (event: string, data: unknown) => { - try { - controller.enqueue(frame(event, data)); - } catch { - /* client gone — the install continues, it is the server's job now */ - } - }; - - // Said before the clone starts, because the name is derived rather than typed and this is the - // only place anyone can see what it became. - send('step', { step: `installing "${appName}" from ${url}` }); - - let result: PluginActionResult; - try { - result = await installPlugin( - appName, - async (step) => send('step', { step }), - userId, - async (line) => send('log', { line }), - url, - ); - } catch (err) { - result = { ok: false, appName, steps: [], error: err instanceof Error ? err.message : String(err) }; - } - send('done', result); - try { - controller.close(); - } catch { - /* already closed */ - } - }, - }); - - return new Response(stream, { - headers: { - 'content-type': 'text/event-stream', - 'cache-control': 'no-cache', - 'x-accel-buffering': 'no', - }, - }); -}); - pluginsRouter.post('/:appName/:verb/stream', async (ctx) => { const appName = ctx.req.param('appName'); const verb = ctx.req.param('verb'); diff --git a/src/workspaces/officerdev/src/apps/Plugins/InstallFromUrl.tsx b/src/workspaces/officerdev/src/apps/Plugins/InstallFromUrl.tsx index c87137d7..8be0bf78 100644 --- a/src/workspaces/officerdev/src/apps/Plugins/InstallFromUrl.tsx +++ b/src/workspaces/officerdev/src/apps/Plugins/InstallFromUrl.tsx @@ -1,75 +1,109 @@ import { useState } from 'react'; -import { GitBranch, X } from 'lucide-react'; +import { GitBranch, X, Loader2 } from 'lucide-react'; /** * "Install from Git repository" — for a plugin that is in no catalogue and should not be. * * The marketplace was the only way a remote plugin could arrive, so "build one and give it to a friend" - * had no affordance at all. It is also the wrong shape for a plugin built for one person: fofos-admin is - * one website's admin panel, and listing it publicly to install it would be a lie about who it is for. + * had no affordance. It is also the wrong shape for a plugin built for one person: fofos-admin is one + * website's admin panel, and listing it publicly in order to install it would be a lie about who it is for. * - * ── Collapsed until asked for ── + * ── It FETCHES. It does not install ── * - * A permanent URL box at the top of a catalogue reads as the primary way in, and it is not — it is the - * exception, for somebody who already has an address. So it is a button that becomes a field, and folds - * away again when it is done. + * The first version installed, and using it showed why that was wrong: this button is on the grid, the + * progress log is in the detail panel, so the install ran to completion with nothing on screen and the + * plugin simply appeared, already installed. No feedback, and no moment to change your mind. + * + * Cloning is enough to make a plugin real — the platform lists what is on disk, so it shows up as + * Available like any catalogue entry. You then click the card and install it there, with the progress + * and the result the existing flow already renders. One install path, and it is the one people know. * * ── No name field ── * * The name is derived from the repository server-side, because `appName` IS the directory a plugin sits - * in and decides its route, its table prefix and its process name. Offering a box for it would invite - * somebody to type one that disagrees with the address. The first streamed step reports what it became, - * which is the moment it can still be cancelled. + * in and decides its route, its table prefix and its process name. A box for it would invite a name that + * disagrees with the address. What it became is reported back, and named in the confirmation. */ -export const InstallFromUrl = ({ onInstall, busy }: { onInstall: (url: string) => void; busy: boolean }) => { +export const InstallFromUrl = ({ + onFetch, +}: { + onFetch: (url: string) => Promise<{ ok: boolean; appName?: string; error?: string }>; +}) => { const [open, setOpen] = useState(false); const [url, setUrl] = useState(''); + const [busy, setBusy] = useState(false); + const [note, setNote] = useState<{ ok: boolean; text: string } | null>(null); if (!open) { return ( - +
+ {/* The outcome stays until the next attempt. A toast would be gone before somebody who just + pasted a URL has looked up, and "which card is mine" is the question this answers. */} + {note && {note.text}} + +
); } - const submit = () => { + const submit = async () => { const trimmed = url.trim(); if (!trimmed || busy) return; - onInstall(trimmed); - setOpen(false); - setUrl(''); + setBusy(true); + setNote(null); + const result = await onFetch(trimmed); + setBusy(false); + if (result.ok) { + // Named, because the name was derived rather than typed — this is where you find out what to look + // for in the grid. + setNote({ ok: true, text: `${result.appName} added — click it to install` }); + setOpen(false); + setUrl(''); + } else { + setNote({ ok: false, text: result.error ?? 'Could not fetch that repository' }); + } }; return (
- + {busy ? ( + + ) : ( + + )} setUrl(e.target.value)} onKeyDown={(e) => { - if (e.key === 'Enter') submit(); - if (e.key === 'Escape') setOpen(false); + if (e.key === 'Enter') void submit(); + if (e.key === 'Escape' && !busy) setOpen(false); }} placeholder="https://gitea.officer.dev/owner/plugin.git" - className="min-w-0 flex-1 rounded-md border border-duck-dark/10 bg-transparent px-2 py-1 font-mono text-xs outline-none focus:border-duck-dark/30" + className="min-w-0 flex-1 rounded-md border border-duck-dark/10 bg-transparent px-2 py-1 font-mono text-xs outline-none focus:border-duck-dark/30 disabled:opacity-50" /> + {note && !note.ok && {note.text}} diff --git a/src/workspaces/officerdev/src/apps/Plugins/PluginGrid.tsx b/src/workspaces/officerdev/src/apps/Plugins/PluginGrid.tsx index 548fc8ff..9d43f979 100644 --- a/src/workspaces/officerdev/src/apps/Plugins/PluginGrid.tsx +++ b/src/workspaces/officerdev/src/apps/Plugins/PluginGrid.tsx @@ -81,7 +81,7 @@ const Card = ({ entry, search }: { entry: Entry; search: string }) => { }; export const PluginGrid = () => { - const { plugins, available, isLoading, installFromUrl, running } = usePlugins(); + const { plugins, available, isLoading, fetchFromUrl } = usePlugins(); const [params] = useSearchParams(); const tag = params.get('tag'); @@ -115,7 +115,7 @@ export const PluginGrid = () => { {tag && tag !== '__untagged' ? ` in ${tag}` : ''}
- +
diff --git a/src/workspaces/officerdev/src/apps/Plugins/usePlugins.ts b/src/workspaces/officerdev/src/apps/Plugins/usePlugins.ts index 5e8969ea..5fe0b2a1 100644 --- a/src/workspaces/officerdev/src/apps/Plugins/usePlugins.ts +++ b/src/workspaces/officerdev/src/apps/Plugins/usePlugins.ts @@ -279,15 +279,27 @@ export function usePlugins() { ); /** - * Install something that is in no catalogue, from its repository. + * Clone a plugin from its repository so it appears as Available. It does NOT install it. * - * The name is derived server-side from the URL and reported back in the first step, so the person sees - * what it became before the clone starts — it is not a field they filled in, and it decides the route, - * the table prefix and the process name. + * That distinction is the whole design. Installing from here ran to completion with nothing on screen, + * because the progress log lives in the detail panel and this button is on the grid. Fetching puts a + * card in the grid instead, and the card already has an Install button with the streamed progress + * beside it — so there is one install flow rather than two, and the one that exists is the one people + * have already used. */ - const installFromUrl = useCallback( - (url: string) => stream('/api/plugins/install-from-url/stream', 'install', url, { url }), - [stream], + const fetchFromUrl = useCallback( + async (url: string): Promise<{ ok: boolean; appName?: string; error?: string }> => { + const res = await fetch('/api/plugins/fetch-from-url', { + method: 'POST', + headers: { ...getHeaders(), 'content-type': 'application/json' }, + body: JSON.stringify({ url }), + }); + const body = (await res.json().catch(() => ({}))) as { appName?: string; error?: string }; + // Refresh either way: a clone that failed discovery still left something on disk worth showing. + invalidate(); + return { ok: res.ok, appName: body.appName, error: body.error }; + }, + [invalidate], ); // Update-all is a PLAIN POST, not the streaming door the single verbs use. Twelve interleaved step @@ -326,7 +338,7 @@ export function usePlugins() { /** Which verb is in flight, or null. */ running, run, - installFromUrl, + fetchFromUrl, updateAll, updatingAll, updateAllResult,