From cac58ea044336261c7a258e826e11ecbffcc4265 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Padez?= Date: Fri, 28 Aug 2026 23:44:43 +0100 Subject: [PATCH] the git-repo button FETCHES, and the card does the installing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit it installed, and using it showed why that was wrong: the button is on the grid, the progress log is in the detail panel — so a URL install ran to completion with nothing on screen and the plugin simply appeared, already installed. no feedback, and no moment to change your mind about running somebody's code. cloning alone is enough to make a plugin real. `/api/plugins` lists what is ON DISK, so a fetched plugin shows up with `installed: false` — Available, exactly like a catalogue entry — and the existing card, its Install button and its streamed progress do the rest. one install path, and it is the one people have already used. so `install-from-url` and its streaming twin are gone, replaced by `fetch-from-url`, which clones and stops. the streaming route existed only to carry progress into a view that could not render it. the outcome is REPORTED, since that was the complaint: "fofos-admin added — click it to install", naming what the URL resolved to. the name is derived rather than typed — appName IS the directory, deciding the route, the table prefix and the process name — so being told what it became is the only way to know which card is yours. it stays on screen until the next attempt rather than being a toast that is gone before you look up. discovery is re-run after the clone: fetchPlugin checks the manifest EXISTS, discovery checks it parses. a directory failing that still renders in the grid as broken, which is honest, but saying so at fetch time reaches the person while they are still looking at the field they pasted into. verified: fetch 200 with the derived name, the plugin listed installed=false, a second fetch refused with "already on disk" rather than clobbering it, and the icon present in the clone. Co-Authored-By: Claude Opus 5 Officer-Session: 656f5c85-e49a-4f1f-b9f3-042a04b4d043 --- src/servers/api/plugins/router.ts | 152 +++++++----------- .../src/apps/Plugins/InstallFromUrl.tsx | 92 +++++++---- .../src/apps/Plugins/PluginGrid.tsx | 4 +- .../officerdev/src/apps/Plugins/usePlugins.ts | 28 +++- 4 files changed, 139 insertions(+), 137 deletions(-) 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,