diff --git a/plugins/EXTRACTING-A-PLUGIN.md b/plugins/EXTRACTING-A-PLUGIN.md index d630307a..78ee10a6 100644 --- a/plugins/EXTRACTING-A-PLUGIN.md +++ b/plugins/EXTRACTING-A-PLUGIN.md @@ -7,7 +7,9 @@ The runbook, written the day offscale became the first one. Follow it for music, 1. `plugins/offscale/PLUGIN.md` — every decision and why, including the three that reversed 2. `plugins/example/` — the reference implementation, deliberately the smallest real plugin 3. `plugins/offscale/` — the worked example, all four parts -4. `src/servers/plugins/` — the system itself: `manifest`, `discover`, `mount`, `install`, `ecosystem`, `schema`, `generate` +4. `plugins/music/PLUGIN.md` — the MESSY worked example: three pieces that stayed behind, and why each + is a seam rather than a loose end. Read it if your feature has anything the platform also uses. +5. `src/servers/plugins/` — the system itself: `manifest`, `discover`, `mount`, `install`, `ecosystem`, `schema`, `generate` --- @@ -19,7 +21,7 @@ The runbook, written the day offscale became the first one. Follow it for music, file is **refused at discovery, by name**. Non-compliance is unrepresentable, not forbidden. **Every plugin permission is grantable, per role, at read or write.** No `kind`, no `ownerOnly`, no field -of any sort. The platform's answer is uniform; what a grant *means* — whose rows a member sees, whether a +of any sort. The platform's answer is uniform; what a grant _means_ — whose rows a member sees, whether a resource is shared or per-user — is the plugin's own job, in its own queries. **Say `permissions`, never the other word.** It already means three things in this codebase. @@ -80,18 +82,18 @@ pm2 restart officer Then through `/plugins`, watching PM2 and the browser at each step: -| Step | Expect | -| --- | --- | -| install | streamed log; schema applied; sidecar online; route mounted | -| the plugin's API | answers | -| the plugin's screen | renders as a Workspace | -| dock | tile appears | -| permissions page | its permission is listed, read/write/none | -| disable | route 404s, sidecar stops, **tables and rows survive** | -| enable | comes back | -| uninstall | route gone, `pm2 list` loses it, **data still there** | -| `bun db:push` while uninstalled | `No changes detected` — data survives | -| install again | identical to the first install | +| Step | Expect | +| ------------------------------- | ----------------------------------------------------------- | +| install | streamed log; schema applied; sidecar online; route mounted | +| the plugin's API | answers | +| the plugin's screen | renders as a Workspace | +| dock | tile appears | +| permissions page | its permission is listed, read/write/none | +| disable | route 404s, sidecar stops, **tables and rows survive** | +| enable | comes back | +| uninstall | route gone, `pm2 list` loses it, **data still there** | +| `bun db:push` while uninstalled | `No changes detected` — data survives | +| install again | identical to the first install | A normal refresh is enough; the shell is `no-store`. When the log's last line appears, the bundle exists. @@ -110,117 +112,74 @@ A normal refresh is enough; the shell is `no-store`. When the log's last line ap - **The shell output is named for the entrypoint** (`index.gen.html`), and `naming` does not change it. - **A stale generated file** (`Plugins.gen.tsx`, `plugin-schemas.gen.ts`) will fail the typecheck after a contract change. Regenerate rather than hand-edit. +- **Delete the feature's `app-store/catalogue.ts` entry, or its screen goes blank.** `capabilityAvailability` + derives from `sidecar_installs`, and a plugin never gets a row there — its install state is + `plugin_installs`. A leftover catalogue entry therefore makes the capability permanently `unavailable`, + which puts its route into `deniedRoutes` and withholds the dock tile, on a server where the plugin is + installed and healthy. This has now bitten twice: headscale (2026-08-14) and nearly music. The note in + `catalogue.ts` is the one to read. +- **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 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. --- -## Music specifically — READ THIS BEFORE STARTING +## Music is done. What it changed about this runbook -**Music is bigger than offscale, and one part of it has nowhere to go.** Two others did, and the owner cut -both from tonight's scope — see below. -Mapped 2026-08-15. +Extracted 2026-08-15 and verified live through the whole table above. `plugins/music/PLUGIN.md` is the +record; the parts worth carrying forward are already folded into the rules and traps above. -**Decide these yourself. Do not stop to ask.** Every one of them has two defensible answers, the owner has -said either is fine, and a corrected decision is cheap where a stalled extraction is not. Record what you -chose and why — in the manifest, in a comment, in the commit message — and keep going. The failure mode to -avoid is not a wrong call; it is a tree left half-moved with a question attached. +The one thing that generalises: **map what the PLATFORM still needs from your feature before you plan the +split.** Music's boundary was not chosen — it was dictated by two imports pointing the wrong way (a +dashboard widget reaching for `useMusicPlayer`, a cliamp relay reaching for `getMusicServerWsUrl`), and +both were found by reading the import graph rather than by reasoning about what music "is". Offscale had +none, so it came out whole and made the job look cleaner than it is. -### What has no home yet +The three pieces music left behind are `officerdev/src/MusicPlayer/`, `src/servers/api/music/router.ts` +and everything cliamp. Each is documented where it sits. **None of them is work waiting for you** — do +not tidy them into a plugin as a warm-up. -1. ~~**Two websocket providers.**~~ **OUT OF SCOPE — do not touch cliamp.** - - `cliamp` and `cliamp-audio` exist for one thing: running the `cliamp` TUI player on the server and - piping its terminal and its audio (a PulseAudio null sink tapped by `parec`) to the browser. It is a - second, separate playback path and the owner's word is that it is the least important part of music — - fun, wanted eventually, not tonight. - - **This is the whole reason the websocket gap does not block this extraction.** Both sockets are already - inert (routes upgrade into commented-out handlers). Leave `sidecar/music/cliamp-ws.ts`, - `sidecar/music/pulse-audio.ts`, `sidecar/music/asoundrc`, `src/servers/api/cliamp/relay.ts` and the two - providers in `server.tsx` exactly where they are, untouched. If the relay's import of - `getMusicServerWsUrl` is the only thing keeping `api/music/router.ts` alive, leave that file too and - say so — a small documented seam is fine. - - What music actually is: the `/music` screen, the library, and the phone and tablet apps that stream - from it. That is what has to work. - -2. **A global UI overlay.** `MusicPlayerHost` is rendered by `DashboardLayout` on **every route**, not - inside a panel, and gates itself on `can('music')`. A plugin contributes panels and a layout; there is - no "render this everywhere" slot, and inventing one is a platform change. -3. ~~**A dashboard widget.**~~ **OUT OF SCOPE — leave it where it is.** - - `src/workspaces/widgets/MusicPlayer/` sits in a third workspace package and is wired through - `WidgetRegistry`. Plugins cannot contribute widgets and are not going to learn how tonight. Leave the - directory, the registration and the barrel export alone; note the seam in the manifest. - -### What else differs - -- **External binaries**: with cliamp out of scope, what the plugin still needs is `ffmpeg` and `ffprobe`, - for tag reading, cover compression and poster frames. A manifest has no way to declare a host - dependency; note it and move on. (`cliamp`, `parec`, `pulseaudio`, `pactl` and `asoundrc` stay behind - with cliamp.) -- **Range requests**: `stream-audio.ts` does 206 / `Content-Range` / 416 and a custom `X-Audio-Duration`; - the proxy runs at `timeoutSeconds: 1800` for reindexes. Verify `Range` survives the hop. -- **Cross-capability**: `MusicBrowser` calls `/file-browser/ls`, not the music sidecar. -- **A CLI tool**: `scripts/reindex-music.ts` reads `DATA_PATH/music/.server`. -- **Music is the platform's worked example.** `registry.ts` explains `personal` through it and - `registry.test.ts` tests the mechanism *through the music capability*. Those tests must be rewritten - against something else, not deleted. -- **Already half-disabled**: `hono.ts` mount and `schema.ts` export are commented out since 2026-08-13; - `officer_db/src/index.ts:49` is still live. So the tree is mid-migration already. - -### The per-user model is NOT tonight's work - -`personal: ['/favorites', '/now-playing', '/playlists', '/queue']` on music's registry entry, the four -user-scoped tables, and the question of what a member's grant *means* — **all of it stays exactly as it -is. Do not design it, do not improve it, do not think about it.** It will be built inside the plugin -later, which is the whole reason the platform's answer is a uniform read/write and nothing more. - -Two facts that make leaving it alone safe rather than lazy: - -- **The web frontend does not use those routes at all.** Favourites, playlists and now-playing are used - only by the phone and tablet apps. Nothing you can see in a browser depends on them, so nothing in this - extraction can regress by carrying them across untouched. -- `/queue` is in that list and **no such route exists**, in the sidecar or anywhere else. Dead or - aspirational. Carry it as-is; do not investigate. - -Move the routes, the tables and the queries verbatim. `userId` keeps arriving from `X-Officer-User` and -the queries keep scoping by it, exactly as today. - -**The one mechanical consequence you cannot avoid:** `registry.test.ts` tests the `personal` mechanism -*through* the music entry, and `registry.ts` uses music as its worked example in prose. Removing the -entry breaks those tests. Re-anchor them on another entry that has `personal` — the smallest edit that -makes them green and still test the mechanism. That is a test fix, not a redesign. - -### How to decide the three, if you want a default - -Each of these is "close the platform gap" or "leave the piece behind". **Closing the gap is better when -you have the runway**, because every later plugin needs it too — but a music that works with cliamp left -in the platform is worth far more than a perfect design that did not land. - -| Part | Close the gap | Leave it behind | -| --- | --- | --- | -| ~~websockets~~ | — | **out of scope. Do not spend a minute on cliamp.** | -| ~~widget~~ | — | **out of scope. Leave `widgets/MusicPlayer` exactly as it is.** | -| global overlay | add one slot the shell renders from installed plugins | leave `MusicPlayerHost` in `DashboardLayout`, gated as it already is | - -**One open call, then.** The player overlay is the only judgement left, and either answer is fine. - -Whatever you pick, the plugin must **install, enable, disable and uninstall cleanly** at the end. A piece -left in the platform is a documented seam; a piece left dangling is a bug. - -`assertCapabilityTotality` is **not** part of this. It is worth fixing eventually and the cliamp routes -are the live example of the drift it cannot see, but both are someone else's evening. +### The global-overlay question is answered, and the answer is no +Music was the first feature wanting to render on every route. It does not get to, and neither will the +next one: a shell slot for a plugin-provided component reopens "there is no way to export a component", +which is the rule the whole frontend contract rests on. `MusicPlayerHost` stays in `DashboardLayout`, +gated on its plugin's permission so it switches itself off with the plugin. +Reopen this only for a feature where the overlay is the whole product, and expect to argue for it. --- +## Which one next + +No decision has been made. What the tree says, for whoever picks it: + +- **`schema.ts` still lists eight commented plugin schemas** — email, notify, dav, photos, jellyfin, + invoiceshelf, soulseek, vault, wallet. Each line names its tables and the file that defines them, which + is exactly what its extraction needs. +- **`hono.ts` still has fifteen commented mounts.** Same list, roughly. +- **Soulseek is the interesting one**, and not because it is easy: `docs/navigation-audit.md` records its + panels making 37 raw upstream calls, which is the mistake the offscale sidecar exists to avoid. Its + extraction is a rewrite wearing a move's clothes. Say so up front rather than discovering it at 2am. +- **Email and wallet both hold credentials**, so they meet `secret-store` and `service_connections` in a + way neither of the first two did. Read `docs/secret-store.md` first. + ## Still open, platform-wide. Do not rediscover these -- **Websocket providers** — `server.reload({ routes })` proven, never called +- **Websocket providers** — `server.reload({ routes })` proven, never called. No plugin owns a socket yet; + music would have been the first and cliamp being out of scope is what let it pass. - **`assertCapabilityTotality` reads the wrong list** — `Object.keys(handlers)` while Bun serves the route table, and plugin routes are not in `PROTECTED_API_PREFIXES` at all. It belongs in `buildHonoApp()`, now the single place routes are mounted. Security-adjacent; close it before members reach plugin routes. + The live example is the two cliamp sockets: served in the route table, claimed by no capability, and + invisible to the check. Pinned by a test in `registry.test.ts` so it stays a known fact. - **Two dock sources** — the app store keeps its own catalogue; one when it is rebuilt on this - **Offscale's queries scope by caller**, so a granted member sees their own empty list rather than the owner's. Its own job, not the platform's. +- **`protocol.ts` declares `:server` per sidecar.** `music:server` and `headscale:server` are both + still there for plugins that have left. Generalising the union to `` `${string}:server` `` is the fix. +- **`hasPersonalWrites` reads `c.personal` only**, so a plugin declaring the same thing through + `readOnlyWrites` reports `false`. Nothing renders it, so it is dead on the wire. diff --git a/plugins/music/PLUGIN.md b/plugins/music/PLUGIN.md new file mode 100644 index 00000000..f263655a --- /dev/null +++ b/plugins/music/PLUGIN.md @@ -0,0 +1,186 @@ +# Music — the second plugin + +**Status: extracted 2026-08-15.** Written after the fact rather than during, because unlike offscale this +one had no design questions left open — the runbook (`plugins/EXTRACTING-A-PLUGIN.md`) had already decided +everything except one call. This records what moved, what did not, and the two bugs the extraction found. + +Read `plugins/offscale/PLUGIN.md` first. It is the design document for the plugin system; this is a +worked second case, and it is interesting mainly for being the messy one. + +--- + +## What music is + +The `/music` screen, the library index, and the phone and tablet apps that stream from it. The contract +those apps speak is `MUSIC_API.md`, next to this file — it is the reason the sidecar's HTTP shape is not +free to change. + +``` +manifest.ts identity, one permission +api/router.ts re-exports the platform's proxy — see below +sidecar/index.ts the whole /api/music contract (503 lines) +sidecar/indexer.ts the library walker → cache tree + manifest (1079 lines) +sidecar/stream-audio.ts 206 / Content-Range / 416, and X-Audio-Duration +sidecar/nightly-reindex.ts 3am full rebuild, staged and swapped +db/ music_favorites, _playlists, _playlist_items, _now_playing +web/ two panels and a layout; the shell renders the Workspace +scripts/ the reindex CLI, which talks to the sidecar port directly +``` + +--- + +## The three things that stayed, and why + +Offscale left nothing behind. Music leaves three, and calling them seams rather than loose ends only +means each one is written down with what would close it. + +### 1. cliamp — out of scope by decision + +`cliamp` and `cliamp-audio` are a _second_ playback path: the `cliamp` TUI run on the server, with its +terminal and its PulseAudio null sink piped to the browser. The owner's call was that it is the least +important part of music and not worth blocking the extraction on. + +It was already inert before any of this — the two sockets are declared in `server.tsx`'s route table and +upgrade into `handlers` entries that are commented out. So: + +- `src/servers/sidecar/music/` still holds `cliamp-ws.ts`, `pulse-audio.ts`, `asoundrc` and + `cliamp-ws.test.ts`. **Untouched.** +- This plugin's sidecar still serves those sockets, so it imports both modules from + `@@/sidecar/music/`. A plugin importing platform code is ordinary; the reverse would not be. +- `src/servers/api/cliamp/relay.ts` stays, and it is what keeps the next item alive. + +### 2. `src/servers/api/music/router.ts` — kept alive by the relay + +`relay.ts` imports `getMusicServerWsUrl` from it. So the platform's proxy could not move, and this +plugin's `api/router.ts` **re-exports it** rather than building a second one. + +That is not laziness. `createSidecarProxy` learns its port from a one-shot `music:server` event and +subscribes at import. Two proxies would mean two subscribers, both working today, and a `503` on the +first reconnect where only one of them happened to be listening — the same class of failure as the +install-order bug offscale found, and just as invisible from reading. + +### 3. The player — the one open judgement call, and it is decided + +**`officerdev/src/MusicPlayer/` stays in the platform.** The runbook left this open with either answer +acceptable. What decided it was not the overlay but the state: + +> `useMusicPlayer` and `PlayerTrack` are imported from `officerdev` by +> `src/workspaces/widgets/MusicPlayer/`, the dashboard widget — which is _also_ out of scope and stays. +> **The platform cannot import from a plugin.** So the player state stays here whatever is decided about +> the UI around it, and a second copy would mean two audio engines fighting over one pair of speakers. + +Given the state had to stay, splitting the engine and the bar away from the thing they drive would have +left the same seam in a worse place. And moving them needed a shell slot that renders a plugin-provided +component on **every route** — which is exactly the escape hatch this system deleted on purpose. "There +is no way to export a component" is what makes "every plugin route is a Workspace" a property of the +shape rather than a rule someone has to remember, and reopening it for one plugin is a bad trade. + +The seam is inert without the plugin: `MusicPlayerHost` gates on `can('music')`, and `music` is now the +plugin's permission — registered at install, gone at uninstall. + +What stayed with it, and why each: `gapless-engine` (the engine the state drives), `player-time` (the +module-level bridge the lyrics pane meets it through), `useLyricsOpen` and `MusicHeart` + +`useMusicFavorites` (the bar renders a heart), and `shared.ts` — the library vocabulary, which the host +needs a third of and the plugin needs all of. One definition on the host side beats a copy either side +of the boundary drifting apart; `plugins/music/web/shared.ts` re-exports it from the package's declared +`officerdev/MusicPlayer/shared` subpath. + +**What would close it:** the widget learning to come from a plugin. Not the overlay slot — that one +should stay shut. + +--- + +## Two bugs, neither visible from reading + +**The app-store catalogue still listed music, and that would have blanked the screen.** +`capabilityAvailability()` derives from `sidecar_installs`, and a _plugin_ never gets a row there — its +install state is `plugin_installs`. So `music` would have been permanently `unavailable`, which puts +`/music` into `deniedRoutes`: dock tile withheld, screen blank, on a server where the plugin was +installed, enabled and healthy. + +This is the **headscale bug, exactly** — and it is documented six lines above where the music entry sat, +in the same file. Found by reading that note rather than by hitting it again, which is the only reason +it cost minutes instead of an evening. Entry removed. + +**`[test] root = "./src"`, so moving `lyrics.test.ts` into `plugins/` stopped running it silently.** The +count fell by nine and the suite still read green-ish. A test that quietly stops running is worse than +one that fails, and _every_ future extraction would have taken its tests out of the suite the same way. +Root is now the repo. Positional filters cannot fix this — `bun test plugins` matches paths under root, +so it finds `src/servers/plugins/` and not `plugins/`. + +--- + +## Permissions + +One permission, `music`, and the key is deliberately unchanged from the registry entry it replaces — so +every existing `role_capabilities` grant keeps meaning what it meant, and `can('music')` keeps resolving +for the overlay. Renaming it would have been a silent data change. + +The old entry carried `personal: ['/favorites', '/now-playing', '/playlists', '/queue']`. A manifest has +no `personal` field and should not grow one: that is the per-user visibility model, which is the plugin's +own job and explicitly not this extraction's work. They ride across on `readOnlyWrites` instead, because +`isRequestAllowedAtLevel` **concatenates the two lists** — one mechanism under two names. A read grant +therefore permits exactly the four paths it permitted yesterday, and no field was added. + +`/queue` is in that list because it was. No such route exists, in the sidecar or anywhere else. + +`[open]` What a member's grant _means_ is unfinished, and music is where the richer model was always +going to be designed (`plugins/offscale/PLUGIN.md` says so). It is genuinely non-uniform here in a way +offscale's is not: favourites, playlists and now-playing are already per-caller — the sidecar scopes +every one by the `X-Officer-User` header the proxy injects — while the library is one shared index for +the household. So "whose row is this" already has a real answer on one side and not the other. That is a +change inside `db/queries.ts`, not a flag on the manifest. + +--- + +## Host dependencies a manifest cannot declare + +`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. + +**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. + +`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. + +--- + +## Verified on the live server, 2026-08-15 + +The runbook's table, run against `platform.officer.dev` rather than reasoned about. + +| Step | Result | +| ------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------- | +| install | streamed 5 steps; schema applied in 2082ms; `officer-music` online; `/example, /music, /offscale` mounted | +| the API | `/api/music/manifest` 200, `/api/music/favorites` returns per-user JSON | +| range requests | full 200 + `Accept-Ranges`; `bytes=100-199` → **206**, correct `Content-Range`, exactly 100 bytes; unsatisfiable → **416**; `../../etc/passwd` → **400** | +| the screen | route generated in `Plugins.gen.tsx`, panels in the built bundle, `PluginScreen` wraps `WorkspaceView`. Structural — not eyeballed in a browser | +| dock | tile present in `/api/user/capabilities`; `/music` in `routes`, not in `deniedRoutes` | +| permissions page | `music` listed among the grantable | +| disable | route 404s, sidecar `stopped`, **rows survive** | +| enable | 200 again, sidecar online, favourites still there | +| uninstall | route 404s, **absent from pm2**, ecosystem entry removed, **rows survive** | +| `bun db:push` while uninstalled | **`No changes detected`**, rows survive | +| install again | byte-identical steps, and a **restore** — the seeded favourite and playlist came back | +| `pm2 restart officer` | boots clean, all three plugins mount, music answers 200 | + +Seeded rows and the audio fixture were removed afterwards; `~/Music` was deleted again, since it did not +exist before. + +**Music is left INSTALLED and enabled.** It had been switched off since 2026-08-13, so this restores it. + +--- + +## Still open + +- **`hasPersonalWrites` reads `c.personal` only**, so the permissions API reports `false` for a plugin + that declares the same thing through `readOnlyWrites`. Nothing renders the field, so it is dead on the + wire — noted rather than fixed. +- **Two dock sources.** The app store keeps its own catalogue while the plugin system builds tiles from + manifests, and the self endpoint concatenates both. One when the store is rebuilt on the plugin system. +- **`src/servers/sidecar/protocol.ts` still declares `music:server`** per sidecar. Generalising the union + to `` `${string}:server` `` is the better fix and is pending for the whole protocol. +- **The cliamp sockets are claimed by no capability**, and are served. Now pinned by a test in + `registry.test.ts` rather than left to be rediscovered — closing it is the totality work.