Report a failed plugin reload instead of exit 0

apps/cli/src/commands/plugin.ts

@@ -1653,14 +1653,16 @@ export function registerPluginCommands(
           if (!result.ok) process.exit(1);
           return;
         }
-        if (!result.ok) exitWithError(result);
+        // A failed reload still carries the inventory: print the targeted
+        // entries (status and detail) before the error and the exit code.
         const reloaded =
           id === undefined
             ? (result.plugins ?? [])
             : (result.plugins ?? []).filter((entry) => entry.id === id);
         for (const entry of reloaded) {
           printPlugin(entry);
         }
+        if (!result.ok) exitWithError(result);
       }),
     );
 

apps/server/src/routes/plugins.ts

@@ -450,8 +450,14 @@ export function registerPluginRoutes(
 
   app.post("/plugins/reload", async (context) => {
     const id = context.req.query("id") ?? undefined;
-    await plugins.reload(id);
-    return context.json({ ok: true, plugins: plugins.list() });
+    const outcome = await plugins.reload(id);
+    // A reload that left a targeted plugin without its current sources
+    // running (degraded after a hung service, or the previous instance kept)
+    // is a failure the caller must see: `bb plugin reload` exits 1 on it and
+    // the dev loop logs it. The inventory rides along so the status detail
+    // is visible either way.
+    if (!outcome.ok) return context.json(outcome, 422);
+    return context.json(outcome);
   });
 
   app.post("/plugins/:id/enable", async (context) => {

apps/server/src/services/plugins/plugin-activation.ts

@@ -55,7 +55,8 @@ interface PluginActivationContext {
   withArtifactLock: <T>(key: string, fn: () => Promise<T>) => Promise<T>;
   withLifecycleLock: <T>(id: string, fn: () => Promise<T>) => Promise<T>;
   disposeOne: (id: string) => Promise<void>;
-  loadOne: (row: InstalledPluginRow) => Promise<void>;
+  /** Resolves the load problem, or null once the row's sources are loaded. */
+  loadOne: (row: InstalledPluginRow) => Promise<string | null>;
   restoreRegistration: (row: InstalledPluginRow) => void;
   provenanceForRow: (row: InstalledPluginRow) => PluginProvenance;
   registrationMatchesForActivation: (

apps/server/src/services/plugins/plugin-registration.ts

@@ -86,7 +86,8 @@ interface PluginRegistrationContext {
   bundledPlugins: readonly BundledPluginRegistration[];
   withLifecycleLock: <T>(id: string, fn: () => Promise<T>) => Promise<T>;
   disposeOne: (id: string) => Promise<void>;
-  loadOne: (row: InstalledPluginRow) => Promise<void>;
+  /** Resolves the load problem, or null once the row's sources are loaded. */
+  loadOne: (row: InstalledPluginRow) => Promise<string | null>;
   statuses: ReadonlyMap<
     string,
     { status: PluginRuntimeStatus; detail: string | null }

apps/server/src/services/plugins/plugin-runtime.ts

@@ -321,6 +321,12 @@ const DEV_BUILD_PROBLEM_LABELS: Record<PluginDevBuildKind, string> = {
   host: "host bundle build failed",
 };
 
+/**
+ * Suffix on a reload problem when the new sources did not load and the
+ * previous instance keeps serving (status stays "running").
+ */
+const PREVIOUS_INSTANCE_KEPT = "the previous instance is still running";
+
 const DEFAULT_LOAD_TIMEOUT_MS = 30_000;
 const DEFAULT_SERVICE_STOP_TIMEOUT_MS = 5_000;
 const DEFAULT_SERVICE_RESTART_BASE_MS = 1_000;
@@ -1354,19 +1360,32 @@ export function createPluginRuntime(context: PluginRuntimeContext) {
     }
   }
 
-  async function loadOne(row: InstalledPluginRow): Promise<void> {
+  function hungServicesDetail(hung: ReadonlySet<string>): string {
+    return `service ${[...hung].join(", ")} did not stop`;
+  }
+
+  /**
+   * Load `row`'s current sources. Resolves null when they are now the running
+   * instance (or the plugin stays disabled by the user's switch), else the
+   * reason they are not: a failed first load, a failed reload that kept the
+   * previous instance serving, or a hung service that blocks the load. The
+   * status is recorded either way; the return value lets the caller that
+   * asked for this load (`bb plugin reload`) report the outcome instead of
+   * success (#2029).
+   */
+  async function loadOne(row: InstalledPluginRow): Promise<string | null> {
     // Refresh identity first so even a disabled/incompatible/errored plugin
     // keeps its name, icon, and logo in the list.
     await populateIdentity(row);
     if (!row.enabled) {
       setStatus(row.id, "disabled");
-      return;
+      return null;
     }
     const previous = loaded.get(row.id);
     function failBeforeFactory(
       status: PluginRuntimeStatus,
       detail: string,
-    ): void {
+    ): string {
       // Every non-running outcome must leave a log line: without one, an
       // engines mismatch after a host upgrade leaves the plugin gone with
       // no trace outside the in-memory status (#1915).
@@ -1375,49 +1394,46 @@ export function createPluginRuntime(context: PluginRuntimeContext) {
         logger.warn(
           `plugin ${row.id} reload failed (kept previous instance): ${detail}`,
         );
-      } else {
-        setStatus(row.id, status, detail);
-        logger.warn(`plugin ${row.id} not loaded (${status}): ${detail}`);
+        return `${detail} (${PREVIOUS_INSTANCE_KEPT})`;
       }
+      setStatus(row.id, status, detail);
+      logger.warn(`plugin ${row.id} not loaded (${status}): ${detail}`);
+      return detail;
     }
     const hung = hungServices.get(row.id);
     if (hung !== undefined && hung.size > 0) {
       // A previous instance's service never stopped; loading now would
       // double-start it (design §3: degraded rather than double-starting).
-      const detail = `service ${[...hung].join(", ")} did not stop`;
+      const detail = hungServicesDetail(hung);
       setStatus(row.id, "degraded", detail);
       logger.warn(`plugin ${row.id} not loaded (degraded): ${detail}`);
-      return;
+      return detail;
     }
     try {
       await stat(row.rootDir);
     } catch {
-      failBeforeFactory(
+      return failBeforeFactory(
         "missing",
         `plugin directory not found: ${row.rootDir} (reinstall)`,
       );
-      return;
     }
     let manifest: PluginManifest;
     try {
       manifest = await readPluginManifest(row.rootDir);
     } catch (error) {
-      failBeforeFactory(
+      return failBeforeFactory(
         "error",
         error instanceof Error ? error.message : String(error),
       );
-      return;
     }
     const engineProblem =
       checkEngineRange(manifest) ?? checkPluginSdkRange(manifest);
     if (engineProblem) {
-      failBeforeFactory("incompatible", engineProblem);
-      return;
+      return failBeforeFactory("incompatible", engineProblem);
     }
     const artifactProblem = await packagedBuiltinArtifactProblem(row, manifest);
     if (artifactProblem !== null) {
-      failBeforeFactory("incompatible", artifactProblem);
-      return;
+      return failBeforeFactory("incompatible", artifactProblem);
     }
     // Build candidate assets without publishing them; a failed reload keeps
     // the previous backend and frontend registration sets together.
@@ -1430,8 +1446,7 @@ export function createPluginRuntime(context: PluginRuntimeContext) {
       hostArtifactProblem =
         error instanceof Error ? error.message : String(error);
       if (previous !== undefined) {
-        failBeforeFactory("error", hostArtifactProblem);
-        return;
+        return failBeforeFactory("error", hostArtifactProblem);
       }
     }
     // Branding refresh rides every load too, so `bb plugin reload` picks up a
@@ -1615,7 +1630,9 @@ export function createPluginRuntime(context: PluginRuntimeContext) {
       logger.warn(
         `plugin ${row.id} failed to load: ${statuses.get(row.id)?.detail}`,
       );
-      return;
+      return previous !== undefined
+        ? `${message} (${PREVIOUS_INSTANCE_KEPT})`
+        : message;
     }
     if (hostArtifactProblem !== null) {
       rollbackGeneration?.();
@@ -1640,7 +1657,7 @@ export function createPluginRuntime(context: PluginRuntimeContext) {
       handle.invalidate();
       setStatus(row.id, "error", hostArtifactProblem);
       logger.warn(`plugin ${row.id} failed to load: ${hostArtifactProblem}`);
-      return;
+      return hostArtifactProblem;
     }
     const plugin: LoadedPlugin = {
       manifest,
@@ -1658,7 +1675,8 @@ export function createPluginRuntime(context: PluginRuntimeContext) {
     };
     if (previous !== undefined) {
       await disposePluginInstance(row.id, previous);
-      if ((hungServices.get(row.id)?.size ?? 0) > 0) {
+      const hungAfterDispose = hungServices.get(row.id);
+      if (hungAfterDispose !== undefined && hungAfterDispose.size > 0) {
         loaded.delete(row.id);
         deps.sharedPorts?.clearDeclarationsForOwner(row.id);
         for (const database of handle.databaseHandles.splice(0)) {
@@ -1669,7 +1687,7 @@ export function createPluginRuntime(context: PluginRuntimeContext) {
           }
         }
         handle.invalidate();
-        return;
+        return hungServicesDetail(hungAfterDispose);
       }
     }
     // One map replacement is the registration commit point. Until this line,
@@ -1720,6 +1738,7 @@ export function createPluginRuntime(context: PluginRuntimeContext) {
       );
     }
     logger.info(`plugin ${row.id}@${manifest.version} loaded`);
+    return null;
   }
 
   async function disposePluginInstance(

apps/server/src/services/plugins/plugin-service.ts

@@ -145,6 +145,18 @@ export interface PluginSkillRootContribution {
   rootPath: string;
 }
 
+/**
+ * Result of `reload`. `plugins` is the full inventory after the reload. A
+ * reload fails when any targeted plugin is not running its current sources
+ * afterwards: the new sources did not load (the previous instance keeps
+ * serving), or a service of the previous instance never stopped and the
+ * plugin is degraded with nothing loaded (#2029). A plugin the user disabled
+ * stays disabled and is not a failure.
+ */
+export type PluginReloadOutcome =
+  | { ok: true; plugins: PluginListEntry[] }
+  | { ok: false; error: string; plugins: PluginListEntry[] };
+
 /**
  * `fs.watch` is allowed to omit the changed filename. The dev loop still has
  * to reload in that case; `.` is a non-ignored synthetic path representing an
@@ -256,7 +268,8 @@ export interface PluginService {
     id: string,
     enabled: boolean,
   ): Promise<PluginListEntry | undefined>;
-  reload(id?: string): Promise<void>;
+  /** Reload one plugin, or every plugin; see PluginReloadOutcome. */
+  reload(id?: string): Promise<PluginReloadOutcome>;
   /** Live API handle for a running plugin (used by later phases and tests). */
   getApi(id: string): BbPluginApi | undefined;
   /**
@@ -1610,14 +1623,17 @@ export function createPluginService(deps: PluginServiceDeps): PluginService {
               }
             },
             reloadPlugin: async () => {
-              await withLifecycleLock(row.id, async () => {
+              const problem = await withLifecycleLock(row.id, async () => {
                 const current = getInstalledPlugin(deps.db, row.id);
-                if (current === undefined) return;
+                if (current === undefined) return null;
                 await disposeOne(row.id);
-                await loadOne(current);
+                return loadOne(current);
               });
               await syncCliSkill();
               notifyPluginsChanged();
+              // The dev loop logs a thrown reload as "reload failed: …"
+              // instead of "reloaded" while the plugin is not running.
+              if (problem !== null) throw new Error(problem);
             },
             log: (message) => logger.info(`plugin ${row.id}: ${message}`),
           });
@@ -1824,11 +1840,19 @@ export function createPluginService(deps: PluginServiceDeps): PluginService {
       const rows = listInstalledPlugins(deps.db).filter(
         (row) => id === undefined || row.id === id,
       );
+      const failures: string[] = [];
       for (const row of rows.sort((a, b) => a.id.localeCompare(b.id))) {
-        await withLifecycleLock(row.id, () => loadOne(row));
+        const problem = await withLifecycleLock(row.id, () => loadOne(row));
+        if (problem !== null) {
+          failures.push(`plugin "${row.id}" reload failed: ${problem}`);
+        }
       }
       await syncCliSkill();
       notifyPluginsChanged();
+      const plugins = list();
+      return failures.length === 0
+        ? { ok: true, plugins }
+        : { ok: false, error: failures.join("; "), plugins };
     },
 
     getApi(id) {

apps/server/src/services/skills/builtin-skills/bb-cli/SKILL.md

@@ -858,7 +858,9 @@ them by mixing ink into canvas), the `--primary` accent, the secondary text tier
     repository subdirectory for a nested plugin, the semver range with its tag
     prefix and resolved tag for a Git range install, engine ranges, install
     time, integrity/registry details, and recent activation history.
-  - `bb plugin enable|disable <id>`, `bb plugin reload [id]`,
+  - `bb plugin enable|disable <id>`, `bb plugin reload [id]` (exits 1 when a
+    reloaded plugin does not come up on its current sources: the previous
+    instance was kept, or it is degraded because a service ignored its abort),
     `bb plugin remove <id>` (deletes the plugin's settings, secrets, and
     schedules; managed git/npm files are deleted, local path sources stay on
     disk, builtin removals are remembered).

apps/server/test/services/plugins/plugin-background.test.ts

@@ -273,18 +273,70 @@ describe("plugin background services", () => {
       `,
     });
     await service.installPath(rootDir);
-    await service.reload("stubborn");
+    const outcome = await service.reload("stubborn");
     const entry = service.list().find((p) => p.id === "stubborn");
     expect(entry?.status).toBe("degraded");
     expect(entry?.statusDetail).toContain("service socket did not stop");
     // Not re-loaded: that would double-start the hung service.
     expect(service.getApi("stubborn")).toBeUndefined();
+    // The plugin is unusable after this reload (#2029): the outcome must say
+    // so instead of resolving as success while `bb stubborn` is gone.
+    expect(outcome).toEqual({
+      ok: false,
+      error: 'plugin "stubborn" reload failed: service socket did not stop',
+      plugins: service.list(),
+    });
 
     // Still degraded on a second reload attempt.
-    await service.reload("stubborn");
+    const again = await service.reload("stubborn");
     expect(service.list().find((p) => p.id === "stubborn")?.status).toBe(
       "degraded",
     );
+    expect(again.ok).toBe(false);
+  });
+
+  it("reports a failed reload that kept the previous instance", async () => {
+    const rootDir = await writePlugin(workDir, {
+      name: "bb-plugin-keeper",
+      serverSource: `
+        export default function plugin(bb: any) {
+          bb.cli.register({ name: "keeper", summary: "keeper", run() { return { exitCode: 0, stdout: "ok" }; } });
+        }
+      `,
+    });
+    const installed = await service.installPath(rootDir);
+    expect(installed.status).toBe("running");
+    const healthy = await service.reload("keeper");
+    expect(healthy.ok).toBe(true);
+
+    // A broken edit: the new sources do not load, so the host keeps the
+    // previous instance serving. The reload still did not apply.
+    await writeFile(
+      join(rootDir, "server.ts"),
+      `export default function plugin() { throw new Error("boom on load"); }`,
+    );
+    const outcome = await service.reload("keeper");
+    const entry = service.list().find((p) => p.id === "keeper");
+    expect(entry?.status).toBe("running");
+    expect(entry?.statusDetail).toBe("reload failed: boom on load");
+    expect(service.getApi("keeper")).toBeDefined();
+    expect(outcome.ok).toBe(false);
+    if (outcome.ok) throw new Error("unreachable");
+    expect(outcome.error).toBe(
+      'plugin "keeper" reload failed: boom on load (the previous instance is still running)',
+    );
+
+    // Reloading every plugin reports the same failure; the fixed plugin
+    // reloads cleanly.
+    expect((await service.reload()).ok).toBe(false);
+    await writeFile(
+      join(rootDir, "server.ts"),
+      `export default function plugin(bb: any) { bb.cli.register({ name: "keeper", summary: "keeper", run() { return { exitCode: 0, stdout: "ok" }; } }); }`,
+    );
+    expect((await service.reload("keeper")).ok).toBe(true);
+    expect(
+      service.list().find((p) => p.id === "keeper")?.statusDetail,
+    ).toBeNull();
   });
 
   it("restarts a crashed service with backoff", async () => {