fix(plugin): build the UI with devDependencies so a self-update ships its own bundle
After the 1.5.2 -> 1.5.3 self-update the server ran 1.5.3 while the dashboard still served the 1.5.2 UI. The client build had failed, and `runtime/client-dist` still held the previous version's bundle, so nothing looked broken — the only signal was the Settings page's "UI build v1.5.2 (rebuild client to match)". Cause: the whole client build toolchain (vite, typescript, @vitejs/plugin-react, tailwindcss, @types/*) is a devDependency, and npm reads NODE_ENV=production as --omit=dev (`NODE_ENV=production npm config get omit` -> `dev`). The dashboard server runs with NODE_ENV=production, and every bootstrap descended from it inherits that: the plugin-update chain, and any SessionStart fired by a `claude` running in a tmux pane the server started. So `npm ci` dropped vite, `tsc -b` died on missing type declarations, and buildClient returned "failed" with the stale bundle left in place. `--include=dev` on both install paths — a CLI flag outranks the env, which is the point. Verified by running buildClient with NODE_ENV=production against the installed plugin root: previously "failed", now "built", and the served bundle carries 1.5.3. Also: plugin-update-helper writes its verdict BEFORE the bootstrap handover as well as after. That step retires the process tree the helper hangs off, and the observed run reached the new server with no verdict file at all — the dashboard reported `last_plugin_update: null` for an update that had actually succeeded.
This commit is contained in:
@@ -2138,6 +2138,7 @@ The same shape is used by `GET /status`, `POST /check`, and the `update_status`
|
||||
| Newer release, ordinary checkout | `{update_available:true, situation:"release_available", latest_release:"vX.Y.Z", manual_command:"...git fetch origin --tags && git merge --ff-only vX.Y.Z..."}` | Modal opens titled with the release version, links its notes, and shows **Update now**. |
|
||||
| Newer release, plugin-cache install | `{update_available:true, situation:"plugin_managed", install_kind:"plugin", plugin_marketplace:"sg", plugin_name:"agenthub", manual_command:"claude plugin marketplace update sg && ..."}` | Modal opens WITH **Update now**; `POST /apply` answers `200 {applied:true, mode:"plugin"}` and the detached chain pulls the new version and re-bootstraps. The modal notes that an already-open Claude Code session still needs `/reload-plugins` for new skills/commands. |
|
||||
| Plugin chain ran but nothing was published | `{last_plugin_update:{ok:true, step:"already_current"}}` | The old server keeps running, untouched; no restart, no error banner. |
|
||||
| Plugin chain mid-handover | `{last_plugin_update:{ok:true, step:"bootstrap", note:"the new plugin version is installed; the server is being handed over"}}` | Written BEFORE the handover on purpose: that step retires the process tree the helper hangs off, and a helper killed there would otherwise leave `last_plugin_update: null` for an update that succeeded. Overwritten by `done` when the helper survives. |
|
||||
| Plugin chain failed (no `claude` on PATH, `npm ci` broke, bootstrap died) | `{last_plugin_update:{ok:false, step:"resolve_claude"\|"plugin_update"\|"bootstrap", error:"..."}}` | Danger banner in the modal naming the failed step. A `bootstrap` failure means the old server was already stopped, so the verdict says to run `/agenthub-update` to recover — and it is read back by whichever server comes up next, because it lives outside the cache. |
|
||||
| Plugin `/apply` with nothing newer published | 409 `{reason:"plugin_managed", release_check:"ok"}` | No chain is spawned. Message names the newest release and points at `/agenthub-update` to force a refresh. |
|
||||
| Plugin `/apply` after a failed release check | 409 `{reason:"plugin_managed", release_check:"unreachable"\|"no_local_version"\|...}` | No chain is spawned, and the message says the check did not succeed rather than claiming nothing newer exists — an unknown reported as a negative is how a broken check looks identical to an up-to-date install. |
|
||||
|
||||
+1
-1
@@ -134,7 +134,7 @@ waits on an install:
|
||||
4. **Install** — `npm install --omit=dev --ignore-scripts` into the runtime dir (never into the plugin cache, which is garbage-collected and replaced on every update). `--ignore-scripts` keeps the root `postinstall` from pulling the whole Vite client toolchain.
|
||||
5. **Legacy hook cleanup** — see above.
|
||||
6. **CLI** — write the `~/.local/bin/agenthub` launcher, never clobbering a `agenthub` the bootstrap did not write.
|
||||
7. **UI build** — `client/` is copied into the runtime dir and built there (`npm install && npm run build`), landing in `runtime/client-dist` — the same directory the server serves from. Skipped when a bundle for the current version already exists; a failure here does not fail the bootstrap (API and MCP still work, and `/agenthub-open` can retry) but does mean `/run` 404s until it's fixed.
|
||||
7. **UI build** — `client/` is copied into the runtime dir and built there (`npm ci --include=dev`, falling back to `npm install --include=dev`, then `npm run build`), landing in `runtime/client-dist` — the same directory the server serves from. `--include=dev` is required, not tidy: the entire build toolchain (`vite`, `typescript`, `@vitejs/plugin-react`, `tailwindcss`, `@types/*`) is a devDependency, and npm reads an inherited `NODE_ENV=production` as `--omit=dev`. Every bootstrap descended from the running server carries that env — the self-update chain, and any `SessionStart` fired by a `claude` inside a tmux pane the server started — so without the flag the install drops `vite` and the build dies on missing types. Skipped when a bundle for the current version already exists; a failure here does not fail the bootstrap (API and MCP still work, and `/agenthub-open` can retry) but does mean `/run` 404s until it's fixed — and when a bundle from the PREVIOUS version is still on disk the failure is quieter still: the dashboard keeps serving the old UI against the new API, visible only as the Settings page's `UI build vX (rebuild client to match)` line.
|
||||
8. **Server** — on Linux with `systemd --user` available, install/refresh a
|
||||
`agenthub.service` unit (see [Auto-start on boot](#auto-start-on-boot)) instead
|
||||
of a bare spawn, so the server is supervised and comes back after a reboot
|
||||
|
||||
+11
-1
@@ -73,7 +73,17 @@ function buildClient({
|
||||
// tarball against the lockfile's integrity hash, so a bad cache entry is
|
||||
// refetched instead of silently reused. It needs a lockfile that matches
|
||||
// package.json — fall back to `install` when the copy has none or has drifted.
|
||||
const npmFlags = ["--no-audit", "--no-fund"];
|
||||
//
|
||||
// `--include=dev` is not optional here: the whole build toolchain (vite,
|
||||
// typescript, @vitejs/plugin-react, tailwindcss, @types/*) lives in the
|
||||
// client's devDependencies, and npm reads `NODE_ENV=production` as
|
||||
// `--omit=dev`. Every bootstrap descended from the running server inherits
|
||||
// that NODE_ENV — the self-update chain and any SessionStart fired from a
|
||||
// tmux pane the server started — so the install silently dropped `vite`,
|
||||
// `npm run build` failed, and the PREVIOUS bundle stayed in place: the
|
||||
// dashboard kept serving the old version's UI while the API was already the
|
||||
// new one. An explicit CLI flag outranks the env, which is the point.
|
||||
const npmFlags = ["--no-audit", "--no-fund", "--include=dev"];
|
||||
if (
|
||||
!exec(NPM, ["ci", ...npmFlags], src, stdio) &&
|
||||
!exec(NPM, ["install", ...npmFlags], src, stdio)
|
||||
|
||||
@@ -157,6 +157,18 @@ function main() {
|
||||
}
|
||||
|
||||
log(`new plugin root: ${newRoot}`);
|
||||
// Write the verdict BEFORE the handover, not only after it. The step below
|
||||
// retires the process tree this helper hangs off, and an observed run reached
|
||||
// the new server with no verdict file at all — so the dashboard reported
|
||||
// `last_plugin_update: null` for an update that had in fact succeeded. A
|
||||
// pre-written record is overwritten by the real outcome when this survives,
|
||||
// and is the outcome when it does not.
|
||||
record({
|
||||
ok: true,
|
||||
step: "bootstrap",
|
||||
plugin_root: newRoot,
|
||||
note: "the new plugin version is installed; the server is being handed over",
|
||||
});
|
||||
try {
|
||||
// Foreground and blocking: this stops the old server (us), waits for the
|
||||
// ports, then starts the new one. It must finish before we record success.
|
||||
|
||||
@@ -391,6 +391,21 @@ describe("buildClient dependency install", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("installs devDependencies explicitly, because NODE_ENV=production omits them", () => {
|
||||
// The whole client build toolchain (vite, typescript, @types/*) is a
|
||||
// devDependency, and npm turns NODE_ENV=production into --omit=dev. Every
|
||||
// bootstrap descended from the running server inherits that env, so the
|
||||
// install dropped vite, `npm run build` failed, and the dashboard kept
|
||||
// serving the PREVIOUS version's bundle after a self-update.
|
||||
resetRuntime();
|
||||
const calls = [];
|
||||
buildClient({ rt: RT, root: fakeRoot(), exec: fakeExec(calls) });
|
||||
|
||||
for (const call of calls.filter((c) => c.startsWith("ci") || c.startsWith("install"))) {
|
||||
assert.match(call, /--include=dev/, `dependency install must include dev deps: ${call}`);
|
||||
}
|
||||
});
|
||||
|
||||
it("fails without building when neither install path works", () => {
|
||||
resetRuntime();
|
||||
const calls = [];
|
||||
|
||||
Reference in New Issue
Block a user