Report a failed plugin reload instead of exit 0 - #2155
Merged
Conversation
SawyerHood
marked this pull request as ready for review
August 21, 2026 05:59
`bb plugin reload` answered ok:true and exited 0 even when the reload
left the plugin unusable: a previous service that ignored its abort put
the plugin in degraded with nothing loaded (its CLI command gone, its
closed database handles still ticking under the orphaned service), or
the new sources failed to load and the previous instance was kept.
`PluginService.reload` returned void, so neither POST /plugins/reload
nor the CLI could tell.
`loadOne` now resolves the load problem (null once the row's current
sources are running or the plugin stays disabled). `reload` collects the
problems per targeted plugin and returns a PluginReloadOutcome; the route
answers 422 { ok: false, error, plugins } on failure, so the CLI prints
the entries and the reason and exits 1, the dev loop logs "reload
failed", and the app/mobile SDK callers surface the error. The builtin
source watcher's reload throws the same problem. No daemon wire change.
The committed-artifact rebuild in the same issue (#2029 defect 1) is the
documented path-install cache policy and is tracked by #1863.
Co-Authored-By: Claude <noreply@anthropic.com>
SawyerHood
force-pushed
the
bb/fix-2029-plugin-reload-orphans
branch
from
August 21, 2026 16:48
8d19397 to
c7cd438
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What was wrong
bb plugin reloadreported success for a reload that left the plugin unusable.PluginService.reload()returnedvoid,POST /api/v1/plugins/reloadanswered{ ok: true }unconditionally, and the CLI exited 1 only onok: falseor an unknown id. So when a background service ignored its abort signal, the runtime correctly refused to double-start it, marked the plugindegraded, closed the old instance's database handles, and unloaded it (itsbbcommand gone, the orphaned service tickingThe database connection is not openevery second), and the reload still returned exit 0 /ok: true. The same held when the new sources failed to load and the runtime kept the previous instance (runningwithreload failed: …). Issue: #2029 (defect 2). Report: https://get-bb.github.io/reports/issues/2029.htmlDefect 1 in the issue (the host rebuilding
dist/app.*into a path-installed plugin root) is the documented path-install cache policy (isMutableAppBundleStale, covered by existing tests) and is tracked by #1863; this PR does not change it.What changed
apps/server/src/services/plugins/plugin-runtime.ts:loadOne(row)resolves the load problem (string | null) instead ofvoid. Null means the row's current sources are running now, or the plugin stays disabled by the user's switch. Every failure exit returns its reason; a reload that kept the previous instance says so (… (the previous instance is still running)); the two hung-service exits sharehungServicesDetail. Statuses and log lines are unchanged.apps/server/src/services/plugins/plugin-service.ts:reload(id?)returnsPluginReloadOutcome:{ ok: true, plugins }or{ ok: false, error, plugins }whereerrorlists each targeted plugin that did not come up andpluginsis the inventory after the reload. The builtin source-watcher'sreloadPluginthrows the same problem so the dev loop logsreload failed: …instead ofreloaded.apps/server/src/routes/plugins.ts: the route answers422 { ok: false, error, plugins }on failure; success body is unchanged (pluginReloadResponseSchemastill matches). SDK callers (webPluginHealthBanner, mobileuseReloadPlugins) already surface a thrownBbHttpErrorwith the server's message. No host daemon wire change, so noHOST_DAEMON_PROTOCOL_VERSIONbump.apps/cli/src/commands/plugin.ts:bb plugin reloadprints the targeted entries (status and detail) before the error and exit 1;--jsonalready exited 1 onok: false.bb plugin devalready throws on!okand its loop logs it.packages/templates/src/templates/bb-guide-plugins.md) and bb-cli skill (apps/server/src/services/skills/builtin-skills/bb-cli/SKILL.md): document the exit code.plugin-registration.ts,plugin-activation.ts:loadOnedep type follows.How you verified
Tests that fail on
origin/mainand pass here:apps/server/test/services/plugins/plugin-background.test.tsmarks the plugin degraded when a service ignores its abortnow asserts the outcome. Before:AssertionError: expected undefined to deeply equal { ok: false, …(2) }.reports a failed reload that kept the previous instance(new): broken edit →running+reload failed: boom on load, outcomeok: falsewithplugin "keeper" reload failed: boom on load (the previous instance is still running); reload-all reports it; a fixed edit reloadsok: true.apps/server/test/services/plugins/plugin-reload-route.test.ts(new, real app harness): success → 200{ ok: true, plugins }; broken sources → 422{ ok: false, error, plugins: [{ status: "running", statusDetail: "reload failed: boom on load" }] }. Before:AssertionError: expected 200 to be 422.Commands (from the committed tree):
pnpm exec turbo run typecheck --filter=@bb/server --filter=@bb/cli --filter=@bb/templates→Tasks: 6 successful, 6 totalpnpm exec turbo run test --filter=@bb/cli --filter=@bb/templates→Tasks: 8 successful, 8 total(453 + 41 tests)pnpm exec turbo run test --filter=@bb/server→ 1825 passed, 1 failed:test/internal/internal-skill-trees.test.ts(mode: 420vs436), the known local umask-0002 failure unrelated to this change; it passes in CI.Manual repro on my own dev instance with the report's fixture (a path plugin whose
lane-watcherservice capturesbb.storage.database()and ignores abort):Before this change the same reload printed the degraded entry and exited 0 with
"ok": true(report step 3).Fixes #2029
Independent verification
Checked out
bb/fix-2029-plugin-reload-orphans(8d19397, one commit on top of origin/main f6fb434; origin/main has not moved since) in a separate worktree and dev instance.Commands:
pnpm install --frozen-lockfile --prefer-offline && pnpm exec turbo run build(18/18 tasks).git checkout origin/main -- apps/cli/src/commands/plugin.ts apps/server/src/routes/plugins.ts apps/server/src/services/plugins/{plugin-activation,plugin-registration,plugin-runtime,plugin-service}.ts, thenpnpm exec vitest run test/services/plugins/plugin-background.test.ts test/services/plugins/plugin-reload-route.test.tsfromapps/server: 3 failed / 15 passed.marks the plugin degraded when a service ignores its abort:AssertionError: expected undefined to deeply equal { ok: false, …(2) }reports a failed reload that kept the previous instance:TypeError: Cannot read properties of undefined (reading 'ok')atexpect(healthy.ok).toBe(true)answers ok:false with the load problem when the new sources did not load (#2029):AssertionError: expected 200 to be 422git checkout HEAD -- <same files>, same vitest run: 18/18 passed.pnpm exec turbo run typecheck --filter=@bb/server --filter=@bb/cli --filter=@bb/templates:Tasks: 6 successful, 6 total.pnpm exec turbo run test --filter=@bb/server --filter=@bb/cli --filter=@bb/templates: cli 453/453, templates 41/41, server 1825 passed / 1 failed (test/internal/internal-skill-trees.test.tsmode 420 vs 436, the known local umask-0002 failure; unrelated, green in CI).Repro on the fixed branch (own dev instance, report fixture: path plugin whose
lane-watcherservice capturesbb.storage.database()and ignores abort):On main the same reload exits 0 with
ok: true(report step 3). The orphaned service still logsThe database connection is not openevery tick, which is the documented degraded contract and out of scope here.Review notes: root cause (reload outcome never propagated) is fixed in the server, which owns the policy; no host-daemon wire change, so no protocol bump needed. CLI
callPluginsalready passes 422 bodies through, the dev loop catches the thrown reload, and the SDK surfaces the 422 asBbHttpErrorwith the server message (web banner toasts, mobile mutation toasts). Guide and bb-cli skill document the exit code. Minor, non-blocking:pluginReloadResponseSchemain server-contract still types only the success body, so SDK callers see the failure viaBbHttpError.bodyuntyped; webPluginHealthBannerdoes not invalidate the plugin list on error and relies on theplugins-changedbroadcast for the refreshed status. Defect 1 (rebuild into a path plugin root) is intentionally left to #1863.Rebase
Rebased onto origin/main
85eec4da6(was 33 commits behind; still one commit, nowc7cd438b3).git rebase origin/mainapplied with no conflicts. Main's changes in the touched files are orthogonal to this fix:plugin-runtime.tsgained providerinstallRankand per-command settings reads (#2148),plugin-service.tsgained agent-tool presentation for grammar v3 (#2164), and the guide/skill docs gained unrelated lines. None of that touchesloadOne's exits orreload, so thestring | nullload problem and thePluginReloadOutcomeplumbing map onto the new code unchanged.Re-proved on the new base. With the six non-test source files reverted to origin/main,
plugin-background.test.ts+plugin-reload-route.test.ts: 3 failed / 15 passed (expected undefined to deeply equal { ok: false, …(2) },Cannot read properties of undefined (reading 'ok'),expected 200 to be 422); restored: 18/18 passed. From the committed tree:turbo typecheck --filter=@bb/server --filter=@bb/cli --filter=@bb/templates --filter=@bb/host-daemon --filter=@bb/sdk→Tasks: 8 successful, 8 total;turbo test --filter=@bb/cli --filter=@bb/templates→Tasks: 8 successful, 8 total(453 + 41 tests);turbo test --filter=@bb/server→ 1898 passed, 2 failed: the known local umaskinternal-skill-treesfailure and a 5s timeout inplugin-update.test.ts(waits one full interval…) on a box at load average 128 with swap full; that file passes 27/27 when run alone.Independent verification (post-rebase)
Re-verified the rebased head
c7cd438b3(one commit; merge base with origin/main is75d6fc4d4, and the only newer main commit,85eec4da6, is a TestFlight CI change) in a fresh worktree and own dev instance.git merge --no-commit origin/mainis clean. Main's intervening changes toplugin-runtime.ts(#2148 providerinstallRank, per-command settings reads) andplugin-service.ts(#2164 grammar-v3 agent-tool presentation) do not add any exit toloadOne; every exit in the rebasedloadOnereturns its problem (string | null) and the final path returnsnull. No provider-bridge-protocol, provider plugin, or agent-runtime file is touched, so no parity run was needed and noHOST_DAEMON_PROTOCOL_VERSIONbump is required.Commands:
pnpm install --frozen-lockfile --prefer-offline && pnpm exec turbo run build(18/18 tasks).git checkout origin/main -- apps/cli/src/commands/plugin.ts apps/server/src/routes/plugins.ts apps/server/src/services/plugins/{plugin-activation,plugin-registration,plugin-runtime,plugin-service}.ts, thenpnpm exec vitest run test/services/plugins/plugin-background.test.ts test/services/plugins/plugin-reload-route.test.tsfromapps/server: 3 failed / 15 passed.marks the plugin degraded when a service ignores its abort:AssertionError: expected undefined to deeply equal { ok: false, …(2) }reports a failed reload that kept the previous instance:TypeError: Cannot read properties of undefined (reading 'ok')answers ok:false with the load problem when the new sources did not load (#2029):AssertionError: expected 200 to be 422git checkout HEAD -- <same files>, same run: 18/18 passed.git status --porcelainempty):pnpm exec turbo run typecheck --filter=@bb/server --filter=@bb/cli --filter=@bb/templates --filter=@bb/sdk --filter=@bb/host-daemon→Tasks: 8 successful, 8 total.pnpm exec turbo run test --filter=@bb/server --filter=@bb/cli --filter=@bb/templates: cli 453/453, templates 41/41, server 1899 passed / 1 failed (test/internal/internal-skill-trees.test.tsmode 420 vs 436, the known local umask-0002 failure; unrelated, green in CI).c7cd438b3: all checks pass (Checks, Package Smoke ubuntu + macos, Tests app-1/2/3, integration, packages, server, version check); iOS simulator and Node compat smoke are conditional skips.Repro on the rebased branch (own dev instance, report fixture: path plugin whose
lane-watchercapturesbb.storage.database()and ignores abort):On main the same reload exits 0 with
ok: true(report step 3). Residual, unchanged from the first verification: the orphaned service still logsThe database connection is not openevery tick (documented degraded contract);pluginReloadResponseSchematypes only the success body, so SDK callers see the 422 as aBbHttpErrorwith the server message; defect 1 (rebuild into a path plugin root) is left to #1863.