diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 3ca65a39..95d8060c 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -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. | diff --git a/docs/PLUGINS.md b/docs/PLUGINS.md index 195573d4..5f33a161 100644 --- a/docs/PLUGINS.md +++ b/docs/PLUGINS.md @@ -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 diff --git a/scripts/plugin-open.js b/scripts/plugin-open.js index 0a1c0c1f..b600fdff 100644 --- a/scripts/plugin-open.js +++ b/scripts/plugin-open.js @@ -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) diff --git a/scripts/plugin-update-helper.js b/scripts/plugin-update-helper.js index 5e31ccf2..f0870768 100644 --- a/scripts/plugin-update-helper.js +++ b/scripts/plugin-update-helper.js @@ -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. diff --git a/server/__tests__/plugin-bootstrap.test.js b/server/__tests__/plugin-bootstrap.test.js index 401da399..42884f40 100644 --- a/server/__tests__/plugin-bootstrap.test.js +++ b/server/__tests__/plugin-bootstrap.test.js @@ -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 = [];