diff --git a/docs/offscale-plugin.md b/docs/offscale-plugin.md index 3a3a64b5..89e7c048 100644 --- a/docs/offscale-plugin.md +++ b/docs/offscale-plugin.md @@ -131,46 +131,77 @@ costume of a normal result. --- -## Mounting — generated, then restart +## Mounting — rebuild and swap, at runtime -Three options were considered: +**Runtime mounting, no restart.** This went round twice — C, then B on the belief that Hono could not +mount at runtime, then back — so the reasoning is recorded rather than the conclusion alone. -- **A** — mounted always, refuses when not installed _(what shipped before this work)_ -- **B** — the mount set is decided at start-up, `officer` restarts on install -- **C** — genuinely dynamic, mounted and unmounted at runtime +### What was actually tested -**B is the decision** — and this reverses an earlier call for C, recorded rather than overwritten because -the reasoning moved. +| Router | `app.route()` after serving has begun | +| -------------------------------- | --------------------------------------------------------------------- | +| `SmartRouter` _(Hono's default)_ | **throws** — `Can not add a route since the matcher is already built` | +| `RegExpRouter` | **throws**, same reason | +| `TrieRouter` | works | +| `PatternRouter` | works | -C was chosen first on the requirement that _the platform must not need to know a plugin exists in -advance_. That requirement stands and is met either way: what the platform reads is a **generated** file -listing the installed routers, exactly analogous to `Plugins.tsx` on the frontend. Nothing is hardcoded and -nothing is read from a table at boot — the imports are made concrete at install time. C would buy only the -absence of a restart, and a restart here is close to free: +So adding at runtime is possible, but only by giving up the fast matcher — and Hono has **no API to +remove a route**, which uninstall needs. -**Sidecars are PM2 peers, never children.** Restarting `officer` does not touch them — that property was -fought for (officer used to spawn the agent, so PM2's tree-kill took the owner's chat down on every -restart) and it is exactly what makes this cheap now. What a restart actually costs is websocket -connections, which reconnect, and officer's in-memory session records, which `claude:list` already exists -to recover. +### The approach that solves both -So: install a plugin → write the generated files → `pm2 restart officer` → the frontend rebuilds and the -page refreshes. Close to imperceptible. +Rebuild the whole app from the current plugin set and **reassign the variable**: + +```ts +let app = buildApp(installedPlugins()); // core routes + one .route() per plugin +serve({ fetch: (req, server) => app.fetch(req, server) }); // closure, NOT app.fetch + +// install: app = buildApp([...installed, 'offscale']) +// uninstall: app = buildApp(installed.filter(p => p !== 'offscale')) +``` + +The `fetch` closure reads `app` on every request, so reassigning it **is** the swap. Verified end to end: + +``` +no plugins /offscale/x -> 404 | /core -> 200 +installed /offscale/x -> 200 | /core -> 200 +uninstalled /offscale/x -> 404 | /core -> 200 +``` + +Better than the TrieRouter route on both counts: the default `SmartRouter` is kept, so the fast +`RegExpRouter` path survives — and **uninstall works**, which an add-only API cannot express. + +### The one line that has to change + +`server.tsx:322` is `'/api/*': honoServer.fetch` — a **bound method**, evaluated once at `serve()`. It has +to become `(req, server) => honoServer.fetch(req, server)`, or reassigning the app has no effect at all. +This is the whole mechanical cost. + +### Websockets are a separate table, and still open + +Six providers are declared in **Bun's route table**, not Hono's: `/api/tasks/run/ws`, +`/api/tasks/pipeline/ws`, `/api/terminal/ws`, `/api/chat/ws`, `/api/cliamp/ws`, `/api/cliamp/audio/ws`. +The Hono swap above does not reach them, so a plugin contributing a websocket provider needs +`server.reload({ routes })` — untested here. + +**Not blocking offscale**, which has none. But it is the reason a plugin cannot yet own a socket, and +it is the same seam as the totality bug below: Bun's route table and Hono's are two different lists. ### What this means for `assertCapabilityTotality` -It stays a boot check, which is the happy consequence of B over C — under C it would have had to become a -per-mount transaction. It just has to see the generated plugin routes, since those are what the boot is -mounting. +It can no longer be only a boot check, because the mount set changes after boot. The question moves to +**per rebuild**: `buildApp()` is the one place routes are mounted, so it is the one place to assert that +every mounted route has a permission — and to refuse the swap if one does not. Same invariant, asserted +where mounting actually happens instead of once at start-up. Two things it must survive, both live today: -- The premise in `sidecar-app-store.md` that "every API route stays mounted regardless" is **retired**. A - route belonging to an uninstalled plugin is not mounted at all, so it cannot be reached by anything. +- The premise in `sidecar-app-store.md` that "every API route stays mounted regardless" is **retired**. An + uninstalled plugin's routes are not mounted, so nothing can reach them. - The check is currently **fed the wrong list** — `Object.keys(handlers)` from `server.tsx`, while Bun - serves the _route table_, and the two diverged when plugins were switched off. Generated mounts will make - that gap wider, not narrower, so pointing totality at the route table is a prerequisite for this rather - than a tidy-up beside it. + serves the _route table_, and the two diverged when plugins were switched off. Moving the assertion into + `buildApp()` fixes this by construction for Hono routes, and leaves the websocket table as the part that + still needs pointing at reality. ---