From 0978c911eebbaa999ecade8e082ed74d43066619 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christopher=20B=C3=B6bel?= Date: Thu, 3 Sep 2026 18:04:42 +0200 Subject: [PATCH 1/2] Migrate registerTool to presentation.label for SDK 0.4.16+ `experimental_statusLabels` was folded into `presentation` (labels) in SDK 0.4.16. bb validates registerTool options in the host, not in the SDK bundled into dist/, so the published artifact is rejected at load time on a current bb regardless of which SDK it was built against: plugin advisor failed to load: registerTool: "experimental_statusLabels" was folded into "presentation" (labels) in SDK 0.4.16 (tool "advisor_review") Observed on bb 0.41.0, 466 ms after installing this commit from git:, followed by three further load failures. The option is spread from a constant rather than inlined because the vendored 0.4.2 declarations in types/ do not describe `presentation` yet, so an inline literal trips TypeScript's excess-property check. --- server.ts | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/server.ts b/server.ts index abdef20..c14f171 100644 --- a/server.ts +++ b/server.ts @@ -17,6 +17,16 @@ import { const PLUGIN_ID = "advisor"; const ADVISOR_TOOL = "advisor_review"; + +// Spread rather than inlined: `presentation` replaced `experimental_statusLabels` +// in SDK 0.4.16, but the vendored 0.4.2 declarations in types/ do not describe it +// yet, so an inline literal trips the excess-property check. See the PR notes on +// `bb plugin migrate` for the follow-up that makes this typed. +const ADVISOR_TOOL_PRESENTATION = { + presentation: { + label: { pending: "Consulting advisor", completed: "Consulted advisor" }, + }, +}; const ADVISOR_TITLE_PREFIX = "Advisor · "; /** * Permission modes the reviewer will accept, least privileged first. The mode @@ -1585,10 +1595,7 @@ export default async function plugin(bb: BbPluginApi) { "Run an independent review-only model pass on this thread and return concrete issues before finalizing.", instructions: "For substantial coding work, call advisor_review exactly once after implementation and verification but before the final answer. Address concern/blocker feedback before completing.", - experimental_statusLabels: { - pending: "Consulting advisor", - completed: "Consulted advisor", - }, + ...ADVISOR_TOOL_PRESENTATION, parameters: z.object({ focus: z .string() From eebe4a8c3469d48ea302ab67a59348a9f8fa41b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christopher=20B=C3=B6bel?= Date: Thu, 3 Sep 2026 18:07:43 +0200 Subject: [PATCH 2/2] Add a per-thread advisor switch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The global Advisor setting is all-or-nothing: a thread where the reviewer is noise cannot opt out without turning it off everywhere. This adds a switch beside the harness selector in the composer, for a thread and for a new thread. A thread override wins over the global setting in both directions, so the global value is a default rather than a ceiling. Off means the advisor never acts on its own in that thread; anything explicitly asked for — "Review now", waiting for completion — still runs. Two tables, both appended to the migration list: advisor_thread_settings one row per overridden thread advisor_new_thread_default the choice armed in the new-thread composer The new-thread composer has no thread id to write against, and plugin settings are read-only from the plugin, so the choice is stored and materialized in thread.created. It is single-use: the next user-created thread consumes and clears it. An earlier sticky variant behaved as a second hidden global that shadowed the real setting with no way back. The gate is checked at every entry point that can start a review on the advisor's own initiative, including inside the tool's execute(): bb does not hot-mutate a running provider session's tool set, so advisor_review can still be present after the switch is flipped off, and it must refuse rather than review. It returns an explicit non-approval so the refusal is not read as a pass. Adds 18 tests (101 total), covering both override directions, single-use arming, other plugins' worker threads not consuming the arm, and the refusal in an already-running session. --- app.test.tsx | 176 +++++++++++++++++++++++++++++++ app.tsx | 214 ++++++++++++++++++++++++++++++++++++++ server.test.ts | 251 ++++++++++++++++++++++++++++++++++++++++++++ server.ts | 276 ++++++++++++++++++++++++++++++++++++++++++++++++- 4 files changed, 912 insertions(+), 5 deletions(-) diff --git a/app.test.tsx b/app.test.tsx index f815dcd..5dfdaea 100644 --- a/app.test.tsx +++ b/app.test.tsx @@ -74,6 +74,182 @@ function panel(reviews: unknown[], extra: Record = {}) { }; } +// Looked up by id rather than by position: the switch and the pending-advice +// banner are separate registrations, and an index would silently follow a +// reorder into the wrong one. +const switchCustomization = app.composerCustomizations.find( + (customization) => customization.id === "advisor-switch", +)!; +const toggleSlot = switchCustomization.actions![0]!; +const threadComposer = { + scope: { kind: "thread", threadId: "t1" } as const, +}; +const newThreadComposer = { + scope: { kind: "new-thread", projectId: "p1" } as const, +}; + +describe("advisor thread switch", () => { + it("reports the effective state of a thread that follows the default", async () => { + const slot = renderSlot(toggleSlot, {}, { + composer: threadComposer, + rpc: { + threadToggle: () => ({ + enabled: false, + override: null, + globalEnabled: false, + }), + }, + }); + + const control = await q(slot).findByRole("switch"); + // The switch shows what actually happens in this thread, not whether the + // user picked it — a thread following a disabled default reads as off. + expect(control.getAttribute("aria-checked")).toBe("false"); + expect(control.getAttribute("aria-label")).toBe("Advisor off"); + }); + + it("writes an explicit override for this thread when clicked", async () => { + let stored: { enabled: boolean | null } = { enabled: null }; + const slot = renderSlot(toggleSlot, {}, { + composer: threadComposer, + rpc: { + threadToggle: () => ({ + enabled: stored.enabled ?? true, + override: stored.enabled, + globalEnabled: true, + }), + setThreadToggle: (input) => { + stored = { enabled: input.enabled }; + return { + enabled: input.enabled ?? true, + override: input.enabled, + globalEnabled: true, + }; + }, + }, + }); + + fireEvent.click(await q(slot).findByRole("switch")); + + await waitFor(() => + expect(q(slot).getByRole("switch").getAttribute("aria-checked")).toBe( + "false", + ), + ); + expect( + slot.inspection.rpcCalls.filter((call) => call.method === "setThreadToggle"), + ).toEqual([ + { method: "setThreadToggle", input: { threadId: "t1", enabled: false } }, + ]); + }); + + it("snaps back instead of claiming a switch the server rejected", async () => { + const slot = renderSlot(toggleSlot, {}, { + composer: threadComposer, + rpc: { + threadToggle: () => ({ + enabled: true, + override: null, + globalEnabled: true, + }), + setThreadToggle: () => { + throw new Error("plugin unavailable"); + }, + }, + }); + + fireEvent.click(await q(slot).findByRole("switch")); + + // The advisor still runs, so the surface must not show an "off" switch. + await waitFor(() => expect(q(slot).queryByRole("switch")).toBeNull()); + expect( + (await q(slot).findByRole("button")).getAttribute("title"), + ).toContain("plugin unavailable"); + }); + + it("names what it does on hover without waiting for the native tooltip", async () => { + const slot = renderSlot(toggleSlot, {}, { + composer: threadComposer, + rpc: { + threadToggle: () => ({ + enabled: true, + override: null, + globalEnabled: true, + }), + }, + }); + + expect((await q(slot).findByRole("tooltip")).textContent).toBe("Advisor on"); + // A native `title` alongside it would surface a second, slower duplicate. + expect(q(slot).getByRole("switch").getAttribute("title")).toBeNull(); + }); + + it("arms the choice for the thread a composer without one will create", async () => { + let stored: boolean | null = null; + const slot = renderSlot(toggleSlot, {}, { + composer: newThreadComposer, + rpc: { + newThreadToggle: () => ({ + enabled: stored ?? true, + override: stored, + globalEnabled: true, + }), + setNewThreadToggle: (input) => { + stored = input.enabled; + return { + enabled: input.enabled ?? true, + override: input.enabled, + globalEnabled: true, + }; + }, + }, + }); + + // Singular on purpose: a plural label would promise a standing default, + // and the choice is spent by the one thread this composer creates. + expect((await q(slot).findByRole("tooltip")).textContent).toBe( + "Advisor on for this new thread", + ); + + fireEvent.click(q(slot).getByRole("switch")); + + await waitFor(() => + expect(q(slot).getByRole("switch").getAttribute("aria-checked")).toBe( + "false", + ), + ); + // Never the per-thread method: there is no thread to attach a choice to. + expect(slot.inspection.rpcCalls.map((call) => call.method)).not.toContain( + "setThreadToggle", + ); + expect(stored).toBe(false); + }); + + it("renders nothing in a composer scope it has no answer for", async () => { + const slot = renderSlot(toggleSlot, {}, { + composer: { + scope: { + kind: "side-chat", + projectId: "p1", + parentThreadId: "t1", + tabId: "tab1", + childThreadId: null, + } as const, + }, + rpc: { + threadToggle: () => ({ + enabled: true, + override: null, + globalEnabled: true, + }), + }, + }); + + await waitFor(() => expect(slot.container.firstChild).toBeNull()); + expect(slot.inspection.rpcCalls).toEqual([]); + }); +}); + describe("advisor header badge", () => { it("keeps advertising an open blocker after a later turn passes", async () => { // The whole point: a clean turn does not close an earlier finding, and the diff --git a/app.tsx b/app.tsx index 0693fb9..87a07ed 100644 --- a/app.tsx +++ b/app.tsx @@ -27,6 +27,7 @@ type Contract = typeof rpcContract; type BadgeData = PluginRpcResult; type PanelData = PluginRpcResult; type PendingAdviceData = PluginRpcResult; +type ToggleData = PluginRpcResult; type PanelReview = PanelData["reviews"][number]; type PanelIncident = PanelData["incidents"][number]; type ReviewLifecycle = PanelData["lifecycle"]; @@ -437,6 +438,210 @@ function badgeStanding( * into the next turn's instructions, so without this banner the agent changes * course and the human is never told why. */ +/** + * The switch's state for whichever composer mounted it: a thread's own choice, + * or the default a newly created thread will inherit. Separate from + * `useThreadAdvisor` because the new-thread variant has no thread id to key a + * fetch or a realtime signal on, and because it is the one surface here whose + * value moves when the *global* setting changes. + */ +function useAdvisorToggle(threadId: string | null, forNewThread: boolean) { + const rpc = useRpc(); + const [data, setData] = useState(null); + const [error, setError] = useState(null); + + const load = useCallback(async () => { + try { + setError(null); + if (forNewThread) { + setData(await rpc.call("newThreadToggle", null)); + } else if (threadId !== null) { + setData(await rpc.call("threadToggle", { threadId })); + } + } catch (caught) { + setError(caught instanceof Error ? caught.message : String(caught)); + } + }, [rpc, threadId, forNewThread]); + + useEffect(() => { + void load(); + }, [load]); + + useRealtime( + "thread-changed", + useCallback( + (payload: unknown) => { + if (threadId === null) return; + const target = + typeof payload === "object" && payload !== null + ? (payload as { threadId?: unknown }).threadId + : undefined; + if (target === threadId) void load(); + }, + [threadId, load], + ), + ); + + // A changed global setting, and the new-thread default, carry no thread id — + // so neither can arrive on the channel above, yet both move what this switch + // should be showing. + useRealtime( + "advisor-settings-changed", + useCallback(() => { + void load(); + }, [load]), + ); + + // Signals are fire-and-forget, so anything published while the socket was + // down is gone. Reconcile on every transition back to `connected`. + const connection = useRealtimeConnectionState(); + const [wasConnected, setWasConnected] = useState(connection === "connected"); + useEffect(() => { + if (connection !== "connected") { + setWasConnected(false); + return; + } + if (wasConnected) return; + setWasConnected(true); + void load(); + }, [connection, wasConnected, load]); + + return { data, error, reload: load }; +} + +/** + * The advisor's switch, mounted in the composer's action row beside the harness + * controls. It is a `role="switch"` rather than a command button because it + * advertises a standing state — "does the advisor run here" — that has to stay + * readable without clicking it. + * + * In a composer that has no thread yet it arms the choice for the thread that + * composer is about to create — so the decision can be made before the first + * message rather than after the advisor has already reviewed a turn — and that + * thread spends it. It is not a second persistent default; the plugin's own + * "Enable advisor" setting is the only one of those. + */ +function AdvisorThreadToggle() { + const composer = useComposer(); + const rpc = useRpc(); + const scope = composer.scope; + const threadId = scope.kind === "thread" ? scope.threadId : null; + const forNewThread = scope.kind === "new-thread"; + const { data, error, reload } = useAdvisorToggle(threadId, forNewThread); + // Holds the requested value until the server confirms it, so the switch does + // not sit in its old position for a full round trip. + const [pending, setPending] = useState(null); + const [writeError, setWriteError] = useState(null); + + const state = data; + + useEffect(() => { + // Any server-confirmed state supersedes the optimistic one, including a + // change made from another client or the CLI. + setPending(null); + }, [state?.enabled, state?.override]); + + const toggle = useCallback(async () => { + if (state === null) return; + const next = !(pending ?? state.enabled); + setPending(next); + setWriteError(null); + try { + if (forNewThread) { + await rpc.call("setNewThreadToggle", { enabled: next }); + } else if (threadId !== null) { + await rpc.call("setThreadToggle", { threadId, enabled: next }); + } + } catch (caught) { + // Snapping back is the point: a switch left in the position the user + // asked for would claim the advisor is off while it still runs. + setPending(null); + setWriteError(caught instanceof Error ? caught.message : String(caught)); + } + await reload(); + }, [rpc, threadId, forNewThread, state, pending, reload]); + + if (threadId === null && !forNewThread) return null; + + const problem = writeError ?? error; + if (problem) { + return ( + + ); + } + + // Nothing to show until the state is known: a switch that renders "off" while + // it loads would misreport a thread the advisor is actually running in. + if (state === null) return null; + + const enabled = pending ?? state.enabled; + // The new-thread wording says "this thread" on purpose: the choice is spent + // by the thread this composer creates, so a plural label would promise a + // standing default the switch does not keep. + const hint = forNewThread + ? enabled + ? "Advisor on for this new thread" + : "Advisor off for this new thread" + : enabled + ? "Advisor on" + : "Advisor off"; + + return ( + + + {/* Paint-only hover hint. `title` is deliberately absent here: the native + tooltip would arrive a second later and duplicate this one. */} + + {hint} + + + ); +} + function AdvisorComposerBanner() { const composer = useComposer(); const navigate = useBbNavigate(); @@ -1302,4 +1507,13 @@ export default definePluginApp((app) => { scopes: ["thread"], banners: [{ id: "pending-advice", chrome: "bare", component: AdvisorComposerBanner }], }); + + // Registered separately from the banner: the switch belongs in the new-thread + // composer too, while a pending-advice banner there would have no thread to + // report on. + app.composer.customize({ + id: "advisor-switch", + scopes: ["thread", "new-thread"], + actions: [{ id: "thread-toggle", component: AdvisorThreadToggle }], + }); }); diff --git a/server.test.ts b/server.test.ts index 937e559..2552351 100644 --- a/server.test.ts +++ b/server.test.ts @@ -127,6 +127,257 @@ END_ADVISOR_RESULT`); }); }); +describe("per-thread advisor switch", () => { + const PASS_OUTPUT = `ADVISOR_RESULT +severity: pass +summary: Looks correct +details: +none +END_ADVISOR_RESULT`; + + it("reports a thread with no choice of its own as following the default", async () => { + const { harness } = await loadAdvisor(PASS_OUTPUT); + + expect(await harness.callRpc("threadToggle", { threadId: "thread-primary" })) + .toEqual({ enabled: true, override: null, globalEnabled: true }); + + await harness.setSettings({ enabled: false }); + + expect(await harness.callRpc("threadToggle", { threadId: "thread-primary" })) + .toEqual({ enabled: false, override: null, globalEnabled: false }); + }); + + it("withholds the tool and instructions from a thread switched off", async () => { + const { harness } = await loadAdvisor(PASS_OUTPUT); + + await harness.callRpc("setThreadToggle", { + threadId: "thread-primary", + enabled: false, + }); + + const gated = await harness.resolveAgentConfiguration(primaryContext); + expect(gated.tools).toEqual([]); + expect(gated.instructions).toBeNull(); + + // A sibling thread that never chose is untouched by the other's override. + const sibling = await harness.resolveAgentConfiguration({ + ...primaryContext, + thread: { ...primaryContext.thread, id: "thread-other" }, + }); + expect(sibling.tools.map((tool) => tool.name)).toEqual(["advisor_review"]); + }); + + it("runs in a thread switched on while the global default is off", async () => { + const { harness } = await loadAdvisor(PASS_OUTPUT); + await harness.setSettings({ enabled: false }); + + expect( + (await harness.resolveAgentConfiguration(primaryContext)).tools, + ).toEqual([]); + + const state = await harness.callRpc("setThreadToggle", { + threadId: "thread-primary", + enabled: true, + }); + expect(state).toEqual({ + enabled: true, + override: true, + globalEnabled: false, + }); + + const opted = await harness.resolveAgentConfiguration(primaryContext); + expect(opted.tools.map((tool) => tool.name)).toEqual(["advisor_review"]); + }); + + it("returns a thread to the default when its override is cleared", async () => { + const { harness } = await loadAdvisor(PASS_OUTPUT); + + await harness.callRpc("setThreadToggle", { + threadId: "thread-primary", + enabled: false, + }); + await harness.callRpc("setThreadToggle", { + threadId: "thread-primary", + enabled: null, + }); + + expect(await harness.callRpc("threadToggle", { threadId: "thread-primary" })) + .toEqual({ enabled: true, override: null, globalEnabled: true }); + expect( + (await harness.resolveAgentConfiguration(primaryContext)).tools.map( + (tool) => tool.name, + ), + ).toEqual(["advisor_review"]); + }); + + it("hands the armed choice to the thread being created", async () => { + const { harness } = await loadAdvisor(PASS_OUTPUT); + + await harness.callRpc("setNewThreadToggle", { enabled: false }); + await harness.emitThreadEvent("thread.created", { + thread: makeThreadResponse({ id: "thread-fresh" }), + }); + + expect(await harness.callRpc("threadToggle", { threadId: "thread-fresh" })) + .toEqual({ enabled: false, override: false, globalEnabled: true }); + expect( + ( + await harness.resolveAgentConfiguration({ + ...primaryContext, + thread: { ...primaryContext.thread, id: "thread-fresh" }, + }) + ).tools, + ).toEqual([]); + }); + + it("spends the armed choice on one thread and no more", async () => { + const { harness } = await loadAdvisor(PASS_OUTPUT); + + await harness.callRpc("setNewThreadToggle", { enabled: false }); + await harness.emitThreadEvent("thread.created", { + thread: makeThreadResponse({ id: "thread-first" }), + }); + await harness.emitThreadEvent("thread.created", { + thread: makeThreadResponse({ id: "thread-second" }), + }); + + // The whole point of the single-use rule: a choice armed once must not keep + // answering for every thread created afterwards. + expect(await harness.callRpc("threadToggle", { threadId: "thread-first" })) + .toEqual({ enabled: false, override: false, globalEnabled: true }); + expect(await harness.callRpc("threadToggle", { threadId: "thread-second" })) + .toEqual({ enabled: true, override: null, globalEnabled: true }); + // And the composer's switch is back to reporting the setting. + expect(await harness.callRpc("newThreadToggle", null)).toEqual({ + enabled: true, + override: null, + globalEnabled: true, + }); + }); + + it("keeps the armed choice safe from other plugins' worker threads", async () => { + const { harness } = await loadAdvisor(PASS_OUTPUT); + + await harness.callRpc("setNewThreadToggle", { enabled: false }); + await harness.emitThreadEvent("thread.created", { + thread: makeThreadResponse({ + id: "worker", + originPluginId: "workflows", + visibility: "hidden", + }), + }); + + // A worker thread appearing between arming and creating must not spend the + // choice the user aimed at their own next thread. + expect(await harness.callRpc("newThreadToggle", null)).toEqual({ + enabled: false, + override: false, + globalEnabled: true, + }); + expect(await harness.callRpc("threadToggle", { threadId: "worker" })) + .toEqual({ enabled: true, override: null, globalEnabled: true }); + }); + + it("leaves threads that already existed on the setting", async () => { + const { harness } = await loadAdvisor(PASS_OUTPUT); + + // thread-primary existed before the switch moved, so it must keep following + // the setting rather than adopt a choice armed for a later thread. + await harness.callRpc("setNewThreadToggle", { enabled: false }); + + expect(await harness.callRpc("threadToggle", { threadId: "thread-primary" })) + .toEqual({ enabled: true, override: null, globalEnabled: true }); + expect( + (await harness.resolveAgentConfiguration(primaryContext)).tools.map( + (tool) => tool.name, + ), + ).toEqual(["advisor_review"]); + }); + + it("never hands the new-thread default to a plugin-owned reviewer", async () => { + const { harness } = await loadAdvisor(PASS_OUTPUT); + + await harness.callRpc("setNewThreadToggle", { enabled: true }); + await harness.emitThreadEvent("thread.created", { + thread: makeThreadResponse({ + id: "thread-advisor", + originPluginId: "advisor", + visibility: "hidden", + }), + }); + + expect(await harness.callRpc("threadToggle", { threadId: "thread-advisor" })) + .toEqual({ enabled: true, override: null, globalEnabled: true }); + }); + + it("reports the new-thread switch as following the global setting until armed", async () => { + const { harness } = await loadAdvisor(PASS_OUTPUT); + + expect(await harness.callRpc("newThreadToggle", null)).toEqual({ + enabled: true, + override: null, + globalEnabled: true, + }); + + await harness.setSettings({ enabled: false }); + expect(await harness.callRpc("newThreadToggle", null)).toEqual({ + enabled: false, + override: null, + globalEnabled: false, + }); + + await harness.callRpc("setNewThreadToggle", { enabled: true }); + expect(await harness.callRpc("newThreadToggle", null)).toEqual({ + enabled: true, + override: true, + globalEnabled: false, + }); + }); + + it("refuses the tool in a thread switched off after the session got it", async () => { + const { harness, spawn } = await loadAdvisor(PASS_OUTPUT); + + // The tool set is handed to a provider session when it starts, so a thread + // switched off mid-session is still holding the tool from when it was on. + await harness.callRpc("setThreadToggle", { + threadId: "thread-primary", + enabled: false, + }); + const answer = await harness.callAgentTool( + "advisor_review", + { focus: "" }, + { threadId: "thread-primary", projectId: "project-test" }, + ); + + expect(spawn).not.toHaveBeenCalled(); + expect(answer).toContain("switched off for this thread"); + // A refusal that reads as a clean bill of health would be worse than no + // gate at all. + expect(answer).toContain("not an approval"); + }); + + it("skips the post-turn review of a thread switched off", async () => { + const { harness, spawn } = await loadAutoAdvisor(PASS_OUTPUT); + + await harness.callRpc("setThreadToggle", { + threadId: "thread-primary", + enabled: false, + }); + await harness.emitThreadEvent("thread.idle", { + thread: makeThreadResponse({ + id: "thread-primary", + projectId: "project-test", + environmentId: "environment-test", + providerId: "codex", + visibility: "visible", + }), + lastAssistantText: "Everything passes.", + }); + + expect(spawn).not.toHaveBeenCalled(); + }); +}); + describe("advisor storage migrations", () => { it("preserves legacy selections and sessions while adding reasoning state", async () => { const host = createFakePluginHost({ pluginId: "advisor" }); diff --git a/server.ts b/server.ts index c14f171..1ecd138 100644 --- a/server.ts +++ b/server.ts @@ -193,11 +193,51 @@ const reviewLifecycleSchema = z.enum([ "unavailable", ]); +/** + * Whether the advisor runs for one thread. `override` is null when the thread + * carries no per-thread choice and simply follows the global setting, so a + * surface can say "following the default" instead of presenting the default as + * something the user picked for this thread. + */ +const threadToggleSchema = z.object({ + enabled: z.boolean(), + override: z.boolean().nullable(), + globalEnabled: z.boolean(), +}); + export const rpcContract = defineRpcContract({ modelConfiguration: { input: z.null(), output: modelConfigurationOutputSchema, }, + threadToggle: { + input: threadTargetSchema, + output: threadToggleSchema, + }, + setThreadToggle: { + input: z + .object({ + threadId: z.string().min(1), + // Null clears the override and returns the thread to the global + // default, which is the only way back once a thread has been pinned. + enabled: z.boolean().nullable(), + }) + .strict(), + output: threadToggleSchema, + }, + /** + * The same switch in a composer that has no thread to attach it to yet. + * `override` is the choice armed for the next thread, which that thread + * consumes; it is not a persistent default. + */ + newThreadToggle: { + input: z.null(), + output: threadToggleSchema, + }, + setNewThreadToggle: { + input: z.object({ enabled: z.boolean().nullable() }).strict(), + output: threadToggleSchema, + }, threadReviews: { input: threadTargetSchema, output: z.object({ @@ -688,6 +728,10 @@ export default async function plugin(bb: BbPluginApi) { let currentSettings = parseRuntimeSettings(await settings.get()); settings.onChange((next) => { currentSettings = parseRuntimeSettings(next); + // Threads that follow the global default have no thread-scoped event to + // wake them, so their toggle would keep showing the previous default until + // a remount. + bb.realtime.publish("advisor-settings-changed", {}); }); const db = bb.storage.database(); @@ -781,6 +825,23 @@ export default async function plugin(bb: BbPluginApi) { // cannot create an unattended agent/reviewer loop. `ALTER TABLE advisor_reviews ADD COLUMN continued_at INTEGER`, `ALTER TABLE advisor_reviews ADD COLUMN auto_continued_at INTEGER`, + // Per-thread override of the global "Enable advisor" switch. Absence of a + // row is meaningful — it is "follow the global setting", not "off" — so the + // default can still be changed for every thread that never opted out. + `CREATE TABLE IF NOT EXISTS advisor_thread_settings ( + primary_thread_id TEXT PRIMARY KEY, + enabled INTEGER NOT NULL CHECK (enabled IN (0, 1)), + updated_at INTEGER NOT NULL + )`, + // The choice the new-thread composer's switch has armed for the next + // thread, if any. Single row by construction, and cleared as soon as a + // thread consumes it — see readPendingNewThreadChoice for why this is not + // a stored default. + `CREATE TABLE IF NOT EXISTS advisor_new_thread_default ( + id INTEGER PRIMARY KEY CHECK (id = 1), + enabled INTEGER NOT NULL CHECK (enabled IN (0, 1)), + updated_at INTEGER NOT NULL + )`, ]); const primaryContexts = new Map(); @@ -829,6 +890,118 @@ export default async function plugin(bb: BbPluginApi) { bb.realtime.publish("thread-changed", { threadId: primaryThreadId }); } + const threadToggleRowSchema = z.object({ enabled: z.number().int() }); + + /** The thread's own choice, or null when it follows the global setting. */ + function readThreadOverride(primaryThreadId: string): boolean | null { + const parsed = threadToggleRowSchema.safeParse( + db + .prepare( + `SELECT enabled FROM advisor_thread_settings WHERE primary_thread_id = ?`, + ) + .get(primaryThreadId), + ); + return parsed.success ? parsed.data.enabled === 1 : null; + } + + function writeThreadOverride( + primaryThreadId: string, + enabled: boolean | null, + ): void { + if (enabled === null) { + db.prepare( + `DELETE FROM advisor_thread_settings WHERE primary_thread_id = ?`, + ).run(primaryThreadId); + return; + } + db.prepare( + `INSERT INTO advisor_thread_settings (primary_thread_id, enabled, updated_at) + VALUES (?, ?, ?) + ON CONFLICT(primary_thread_id) DO UPDATE + SET enabled = excluded.enabled, updated_at = excluded.updated_at`, + ).run(primaryThreadId, enabled ? 1 : 0, Date.now()); + } + + /** + * The one question every gate asks. An override wins in both directions: the + * global setting is the default for threads that never chose, not a ceiling, + * so a single thread can still run the advisor while it is off everywhere + * else. + */ + function advisorEnabledFor(primaryThreadId: string): boolean { + return readThreadOverride(primaryThreadId) ?? currentSettings.enabled; + } + + /** + * The choice armed for the next thread, or null when none is waiting. + * + * Deliberately single-use rather than a stored default: a value that survived + * being applied would be a second global setting shadowing the real one, with + * no way back to it — and it would keep answering for threads created weeks + * later, long after the composer that set it was forgotten. A persistent + * default belongs in the plugin's own settings, which already carry one. + */ + function readPendingNewThreadChoice(): boolean | null { + const parsed = threadToggleRowSchema.safeParse( + db.prepare(`SELECT enabled FROM advisor_new_thread_default WHERE id = 1`).get(), + ); + return parsed.success ? parsed.data.enabled === 1 : null; + } + + function writePendingNewThreadChoice(enabled: boolean | null): void { + if (enabled === null) { + db.prepare(`DELETE FROM advisor_new_thread_default WHERE id = 1`).run(); + return; + } + db.prepare( + `INSERT INTO advisor_new_thread_default (id, enabled, updated_at) + VALUES (1, ?, ?) + ON CONFLICT(id) DO UPDATE + SET enabled = excluded.enabled, updated_at = excluded.updated_at`, + ).run(enabled ? 1 : 0, Date.now()); + } + + /** Read and clear in one step, so exactly one thread can consume a choice. */ + function consumePendingNewThreadChoice(): boolean | null { + const pending = readPendingNewThreadChoice(); + if (pending !== null) writePendingNewThreadChoice(null); + return pending; + } + + function newThreadToggleState() { + const override = readPendingNewThreadChoice(); + return { + enabled: override ?? currentSettings.enabled, + override, + globalEnabled: currentSettings.enabled, + }; + } + + function describeThreadToggle(primaryThreadId: string): string { + const { enabled, override } = threadToggleState(primaryThreadId); + const state = enabled ? "enabled" : "disabled"; + return override === null + ? `${state} (following the default)` + : `${state} (set for this thread)`; + } + + function describeNewThreadToggle(): string { + const { enabled, override } = newThreadToggleState(); + const state = enabled ? "enabled" : "disabled"; + return override === null + ? `${state} (following the default)` + : `${state} (armed in the composer for the next thread only)`; + } + + function threadToggleState(primaryThreadId: string) { + const override = readThreadOverride(primaryThreadId); + return { + enabled: override ?? currentSettings.enabled, + override, + globalEnabled: currentSettings.enabled, + }; + } + function readChainRoot( primaryThreadId: string, chainId: number, @@ -1344,6 +1517,28 @@ export default async function plugin(bb: BbPluginApi) { bb.rpc.register(rpcContract, { modelConfiguration, + threadToggle({ threadId }) { + return threadToggleState(threadId); + }, + + setThreadToggle({ threadId, enabled }) { + writeThreadOverride(threadId, enabled); + publishThreadChanged(threadId); + return threadToggleState(threadId); + }, + + newThreadToggle() { + return newThreadToggleState(); + }, + + setNewThreadToggle({ enabled }) { + writePendingNewThreadChoice(enabled); + // No thread id to scope this to, so the surfaces that show it listen on + // the settings channel instead. + bb.realtime.publish("advisor-settings-changed", {}); + return newThreadToggleState(); + }, + threadReviews({ threadId }) { return { reviews: collapseChains(readThreadRows(threadId)), @@ -1604,6 +1799,14 @@ export default async function plugin(bb: BbPluginApi) { .describe("What changed, what was verified, and any uncertainty the advisor should examine"), }), async execute({ focus }, context) { + // The tool set is not hot-mutated: bb hands it to a provider session when + // that session starts or resumes, so a thread switched off mid-session + // can still be holding this tool from when it was on. Re-reading the + // switch here is what makes "off" actually mean off, rather than "off + // once the session happens to restart". + if (!advisorEnabledFor(context.threadId)) { + return "Advisor is switched off for this thread, so no review ran. This is not an approval — nothing was checked. Finish your work as you normally would."; + } // The gate is mandatory, so it must always answer. An unexpected throw // would surface as a raw tool error rather than the explicit // "unavailable, and this is not approval" contract the agent relies on. @@ -1657,7 +1860,7 @@ export default async function plugin(bb: BbPluginApi) { bb.agents.configure((context) => { const isPluginOwnedThread = context.origin.pluginId !== null; - if (isPluginOwnedThread || !currentSettings.enabled) { + if (isPluginOwnedThread || !advisorEnabledFor(context.thread.id)) { return { tools: [], skills: [] }; } @@ -2376,7 +2579,7 @@ export default async function plugin(bb: BbPluginApi) { } if (toolReviewed) return; if ( - !currentSettings.enabled || + !advisorEnabledFor(thread.id) || !currentSettings.autoReview || !lastAssistantText || thread.visibility === "hidden" || @@ -2442,6 +2645,9 @@ export default async function plugin(bb: BbPluginApi) { db.prepare(`DELETE FROM advisor_sessions WHERE primary_thread_id = ?`).run( thread.id, ); + db.prepare( + `DELETE FROM advisor_thread_settings WHERE primary_thread_id = ?`, + ).run(thread.id); } // The hidden reviewer is archived either way: it is an appliance of the @@ -2453,6 +2659,31 @@ export default async function plugin(bb: BbPluginApi) { } } + /** + * Hand the armed choice to the thread it was armed for, and clear it. Doing + * this at creation rather than lazily in the gate is what keeps the switch + * honest in both directions: threads that already existed are untouched, and + * the next one after this is back to following the plugin's own setting. + */ + bb.events.on("thread.created", ({ thread }) => { + // Reviewer threads and other plugins' worker threads are created through + // this same event. Neither is the thread the user armed the switch for, so + // letting one consume the choice would silently spend it on the wrong + // thread — and hand the advisor's own reviewer the gate it exists to answer. + if (thread.originPluginId !== null) return; + // A thread that somehow already carries a choice keeps it, but the armed + // one is still spent: it was aimed at this thread either way. + const pending = consumePendingNewThreadChoice(); + if (pending === null) return; + if (readThreadOverride(thread.id) === null) { + writeThreadOverride(thread.id, pending); + } + publishThreadChanged(thread.id); + // Snaps every open new-thread composer's switch back to the setting, so it + // never keeps advertising a choice that has already been spent. + bb.realtime.publish("advisor-settings-changed", {}); + }); + bb.events.on("thread.deleted", ({ thread }) => retireThread(thread, true)); bb.events.on("thread.archived", ({ thread }) => retireThread(thread, false)); @@ -2470,13 +2701,47 @@ export default async function plugin(bb: BbPluginApi) { summary: "Show recent advisor reviews", usage: "bb advisor reviews [thread-id]", }, + { + name: "enable", + summary: "Run the advisor in one thread, whatever the global setting is", + usage: "bb advisor enable [thread-id]", + }, + { + name: "disable", + summary: "Stop the advisor in one thread, whatever the global setting is", + usage: "bb advisor disable [thread-id]", + }, + { + name: "follow", + summary: "Drop a thread's override so it follows the global setting again", + usage: "bb advisor follow [thread-id]", + }, ], run(argv, cliContext) { const [command = "status", explicitThreadId] = argv; const threadId = explicitThreadId ?? cliContext.threadId; + + if (command === "enable" || command === "disable" || command === "follow") { + if (!threadId) { + return { + exitCode: 1, + stderr: "Pass a thread id or run from a bb thread.", + }; + } + writeThreadOverride( + threadId, + command === "follow" ? null : command === "enable", + ); + publishThreadChanged(threadId); + return { + exitCode: 0, + stdout: `Thread ${threadId}: ${describeThreadToggle(threadId)}`, + }; + } + if (command === "status") { const target = threadId - ? `\nThread: ${threadId}\nReview lifecycle: ${reviewLifecycle(threadId)}` + ? `\nThread: ${threadId}\nThread advisor: ${describeThreadToggle(threadId)}\nReview lifecycle: ${reviewLifecycle(threadId)}` : ""; const selections = db .prepare( @@ -2499,7 +2764,7 @@ export default async function plugin(bb: BbPluginApi) { .join(", "); return { exitCode: 0, - stdout: `Advisor: ${currentSettings.enabled ? "enabled" : "disabled"}\nAuto review: ${currentSettings.autoReview ? "enabled" : "disabled"}\nAuto continue: ${currentSettings.autoContinue ? "enabled" : "disabled"}\nMachine models: ${modelStatus}${target}`, + stdout: `Advisor default: ${currentSettings.enabled ? "enabled" : "disabled"}\nNew threads: ${describeNewThreadToggle()}\nAuto review: ${currentSettings.autoReview ? "enabled" : "disabled"}\nAuto continue: ${currentSettings.autoContinue ? "enabled" : "disabled"}\nMachine models: ${modelStatus}${target}`, }; } if (command === "reviews") { @@ -2534,7 +2799,8 @@ export default async function plugin(bb: BbPluginApi) { } return { exitCode: 1, - stderr: "Usage: bb advisor status [thread-id]\n bb advisor reviews [thread-id]", + stderr: + "Usage: bb advisor status [thread-id]\n bb advisor reviews [thread-id]\n bb advisor enable|disable|follow [thread-id]", }; }, });