diff --git a/plugins/stewards/DESIGN.md b/plugins/stewards/DESIGN.md index 192f704..671ee70 100644 --- a/plugins/stewards/DESIGN.md +++ b/plugins/stewards/DESIGN.md @@ -1646,19 +1646,31 @@ the board calls it unfiltered, the sidebar calls it with `actionableOnly`, and need is on the page but not in the sidebar's items. **Realtime, not the poll.** The store takes an optional -`onNeedsYouChange(projectId)` callback, wired in `server.ts` to +`onProjectChange(projectId)` callback, wired in `server.ts` to `bb.realtime.publish("needs-you-changed", { projectId })`; the store stays -free of bb. The callback fires from the store's mutation points — a question -filed (`ask`) or answered (`answer`, `answerQuestionOnly`, `answerEscalation`), -an escalation filed route-`me` (`escalate`) or forwarded to the user or -resolved (`reroute`, `resolve`, `approve`, `decline`), a need filed, updated or -closed (`fileNeed`, `closeNeed`, `closeNeeds`, `closeNeedByKey`), and a binding -ended or reopened (`patch`) — so a steward's own tools publish as well as the -page's buttons. A synchronous coalescing guard makes one question filed or +free of bb. The channel string is unchanged from the narrower "needs you" +signal (BB-90), so a server and client built from different images keep +talking across a restart. `patch` compares the binding before and after and +fires when a sidebar/board-visible column changed (`stage`, `phase`, `hold`, +`hold_behind`, `status`, `pending`, `requeue_at`, `advanced_at`, +`deployments`, `title`, `url`, `ending`, `ended_at`, `thread_id`), staying +silent for per-turn bookkeeping (`lastJudgmentAt`, `lastSnapshot`, `wokeAt`, +`awaitingReport`, `missedReports`, `archiveWhenIdle`, `manualDetach`, the +model-check flags) and same-value writes; `addWorker` and `setWorker` fire +too. `note()` writes its queued-notes column without a signal — a note is a +reason for the next judgment turn, and its one visible effect, the "worker +finished" phrase, is published by `setWorker` when the worker's status +changes. The callback also fires from the needs-you mutation points — a +question filed (`ask`) or answered (`answer`, `answerQuestionOnly`, +`answerEscalation`), an escalation filed route-`me` (`escalate`) or forwarded +to the user or resolved (`reroute`, `resolve`, `approve`, `decline`), and a +need filed, updated or closed (`fileNeed`, `closeNeed`, `closeNeeds`, +`closeNeedByKey`). A synchronous coalescing guard makes one question filed or answered publish exactly one event even when the operation nests (`answer` resolves its escalation). Every open sidebar refetches when the event names a -project it shows, and the board page refetches when it names its project; the -half-minute poll remains the fallback. +project it shows, the board page refetches when it names its project, and the +overview (which covers every project) refetches on any event; the half-minute +poll remains the fallback. **The default provider, checked (BB-44 criterion 1).** With `sidebar.threadListProvider` unset it is `__automatic__`, which bb resolves to diff --git a/plugins/stewards/README.md b/plugins/stewards/README.md index dd2c499..f520558 100644 --- a/plugins/stewards/README.md +++ b/plugins/stewards/README.md @@ -448,11 +448,13 @@ server's refusal is shown in the row, and the row refetches on success. The item carries the binding's entity key as `bindingKey`, so a row owns its item; a project-level question with no binding has no row and stays on the Stewards page. The plugin publishes `needs-you-changed { projectId }` on its realtime -channel whenever a question is opened or answered, an escalation is filed with -or forwarded to the user or resolved, a need is filed, updated or closed, or a -binding ends or reopens — including from a steward's own tools — so every open -sidebar and the board page refresh within seconds. The half-minute poll stays -as the fallback. +channel whenever a sidebar- or board-visible binding state changes: a question +opened or answered, an escalation filed with or forwarded to the user or +resolved, a need filed, updated or closed, and a stage, phase, hold, status +report, requeue time, deployment state, title, link, ending or worker starting +or finishing — including from a steward's own tools — so every open sidebar, +the board page and the overview refresh within seconds. The half-minute poll +stays as the fallback. **The footer toggle.** A `ShieldCheck` icon in the sidebar footer (next to Settings) opens a small panel above the footer row with a **Stewards diff --git a/plugins/stewards/app.test.tsx b/plugins/stewards/app.test.tsx index 9d35733..1c0f4a2 100644 --- a/plugins/stewards/app.test.tsx +++ b/plugins/stewards/app.test.tsx @@ -5,7 +5,7 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import { fireEvent, waitFor } from "@testing-library/react"; import { loadPluginApp, renderSlot, type CapturedPluginApp, type RenderedSlot } from "@get-bb/plugin-sdk/testing/app"; -import { NEEDS_YOU_CHANGED } from "./channels.js"; +import { PROJECT_CHANGED } from "./channels.js"; type Need = { id: string; @@ -478,10 +478,10 @@ describe("the stewards sidebar's picker counts and refresh (BB-46)", () => { }, }); await waitFor(() => expect(calls).toBe(1)); - await slot.emitRealtime(NEEDS_YOU_CHANGED, { projectId: "p9" }); + await slot.emitRealtime(PROJECT_CHANGED, { projectId: "p9" }); await new Promise((r) => setTimeout(r, 20)); expect(calls).toBe(1); - await slot.emitRealtime(NEEDS_YOU_CHANGED, { projectId: "p1" }); + await slot.emitRealtime(PROJECT_CHANGED, { projectId: "p1" }); await waitFor(() => expect(calls).toBe(2)); slot.lifecycle.unmount(); }); @@ -504,15 +504,37 @@ describe("the stewards sidebar's picker counts and refresh (BB-46)", () => { await waitFor(() => expect(calls).toBe(1)); // The handler only refetches once the board (and its project id) has rendered. await waitFor(() => expect(slot.getByText("Acme")).toBeTruthy()); - await slot.emitRealtime(NEEDS_YOU_CHANGED, { projectId: "p1" }); + await slot.emitRealtime(PROJECT_CHANGED, { projectId: "p1" }); await waitFor(() => expect(calls).toBe(2)); - await slot.emitRealtime(NEEDS_YOU_CHANGED, { projectId: "p2" }); + await slot.emitRealtime(PROJECT_CHANGED, { projectId: "p2" }); await new Promise((r) => setTimeout(r, 20)); expect(calls).toBe(2); slot.lifecycle.unmount(); }); }); +// BB-90: the overview (Settings → Plugins → Stewards → Status) polled only, so +// it now subscribes to the widened channel too. +describe("the stewards overview's refresh (BB-90)", () => { + it("refetches on a realtime event for any project", async () => { + const captured = await loadApp(); + const section = captured.settingsSections[0]!; + let calls = 0; + const slot = renderSlot(section, {}, { + rpc: { + stewardsOverview: () => { + calls += 1; + return { paused: false, projects: [], lastSweepAt: 0 }; + }, + }, + } as never); + await waitFor(() => expect(calls).toBe(1)); + await slot.emitRealtime(PROJECT_CHANGED, { projectId: "p1" }); + await waitFor(() => expect(calls).toBe(2)); + slot.lifecycle.unmount(); + }); +}); + // BB-78: the Stewards page adopts the sidebar's two-line row, glyph and phrase, // with the Blocked / In flight / Monitors / Recently ended sections. const boardSteward = (over: Record = {}) => ({ diff --git a/plugins/stewards/app.tsx b/plugins/stewards/app.tsx index d6322b9..0b64fe3 100644 --- a/plugins/stewards/app.tsx +++ b/plugins/stewards/app.tsx @@ -31,7 +31,7 @@ import { type PluginThreadListProps, } from "@get-bb/plugin-sdk/app"; import type { rpcContract } from "./server"; -import { NEEDS_YOU_CHANGED } from "./channels.js"; +import { PROJECT_CHANGED } from "./channels.js"; import { buildSidebar, compareBlocked, compareEfforts, compareInflight, deploymentPhrase, effortPhrase, effortSection, effortState, effortTitle, isBusy, type EffortStatus, type SidebarChild, type SidebarChildRole, type SidebarEffort, type SidebarEffortState, type SidebarSection, type SidebarTab } from "./sidebar.js"; import { AUTOMATIC_THREAD_LIST_PROVIDER, BUILTIN_THREAD_LIST_ID, THREAD_LIST_PROVIDER_KEY, stewardsThreadListId } from "./sidebar-provider.js"; @@ -104,7 +104,13 @@ function usePolled(load: () => Promise): { data: T | null; error: string | function useOverview(): { overview: Overview | null; error: string | null } { const rpc = useRpc(); - const { data, error } = usePolled(useCallback(() => rpc.call("stewardsOverview", null), [rpc])); + const { data, error, refetch } = usePolled(useCallback(() => rpc.call("stewardsOverview", null), [rpc])); + // The overview covers every project, so any project's visible change + // refetches it; the half-minute poll stays the fallback. + useRealtime( + PROJECT_CHANGED, + useCallback(() => refetch(), [refetch]), + ); return { overview: data, error }; } @@ -457,9 +463,9 @@ function BoardPage({ subPath }: { subPath: string }) { useEffect(() => { if (wanted === null && boardProjectId !== null) navigate.toPluginPanel(BOARD_PATH, { subPath: boardSubPath(boardProjectId), replace: true }); }, [wanted, boardProjectId, navigate]); - // A steward's question or item shows up here without the half-minute poll. + // A steward's visible change shows up here without the half-minute poll. useRealtime( - NEEDS_YOU_CHANGED, + PROJECT_CHANGED, useCallback( (payload) => { const projectId = payload === null || typeof payload !== "object" ? null : (payload as { projectId?: unknown }).projectId; @@ -1656,11 +1662,11 @@ function StewardsSidebarCore({ const needsByProject = useMemo(() => new Map((data?.projects ?? []).map((p) => [p.projectId, p.needs] as const)), [data]); // The picker counts the yellow rows, so the pill and the sections cannot disagree (BB-76). const counts = useMemo(() => new Map(tabs.map((t) => [t.projectId, t.blocked.filter((e) => e.state === "you").length] as const)), [tabs]); - // A steward's question or item updates the badge and block within seconds; + // A steward's visible change updates the badge and block within seconds; // the half-minute poll stays the fallback. Only a project this sidebar shows // triggers a refetch. useRealtime( - NEEDS_YOU_CHANGED, + PROJECT_CHANGED, useCallback( (payload) => { const projectId = payload === null || typeof payload !== "object" ? null : (payload as { projectId?: unknown }).projectId; diff --git a/plugins/stewards/channels.ts b/plugins/stewards/channels.ts index 37d2f20..d021de5 100644 --- a/plugins/stewards/channels.ts +++ b/plugins/stewards/channels.ts @@ -2,9 +2,16 @@ // imports only the strings and never server code (BB-46). /** - * Published as `{ projectId }` whenever a project's "needs you" set changes: - * a question opened or answered, an escalation forwarded to the user or - * resolved, or a needs item filed, updated or closed. Every sidebar and the - * board page refetch on it; the half-minute poll is the fallback. + * Published as `{ projectId }` whenever a project's sidebar- or + * board-visible steward state changes: the "needs you" set (a question + * opened or answered, an escalation forwarded to the user or resolved, or a + * needs item filed, updated or closed) and the rest of the row (a stage, + * phase, hold, status report, requeue time, deployment state, title, link or + * ending, or a worker starting or finishing). Every sidebar, board page and + * the overview refetch on it; the half-minute poll is the fallback. + * + * The channel string stays `needs-you-changed` (BB-90): it was published + * under that name before the signal widened, and a server and client built + * from different images must keep talking across a restart. */ -export const NEEDS_YOU_CHANGED = "needs-you-changed"; +export const PROJECT_CHANGED = "needs-you-changed"; diff --git a/plugins/stewards/server.test.ts b/plugins/stewards/server.test.ts index ea562b9..52d04cd 100644 --- a/plugins/stewards/server.test.ts +++ b/plugins/stewards/server.test.ts @@ -1,7 +1,7 @@ import Database from "better-sqlite3"; import { createFakePluginHost, makePluginAgentConfigurationContext, makeThreadResponse } from "@get-bb/plugin-sdk/testing"; import { describe, expect, it } from "vitest"; -import { NEEDS_YOU_CHANGED } from "./channels.js"; +import { PROJECT_CHANGED } from "./channels.js"; import { PROJECT_FILE, playbookPath } from "./config.js"; import { comment, crashingPod, issue, pull, warningEvent } from "./fixtures.js"; import { kubectlRefusal, type KubectlResult } from "./kubectl.js"; @@ -9,7 +9,7 @@ import { retiredLessonAnchor, withLessonAnchor } from "./lessons.js"; import { guidanceUnitId } from "./guidance.js"; import { createStewardsPlugin, parseDotenvValue, SECRET_SETTING_NAMES, secretVariable, testHooks, type StewardsDeps } from "./server.js"; import { SOURCE_PLUGIN_IDS, type EntitySnapshot } from "./source-contract.js"; -import { MIGRATIONS, ESCALATED_NEED_KINDS, NEEDS_KINDS, STEWARD_NEED_KINDS } from "./store.js"; +import { MIGRATIONS, ESCALATED_NEED_KINDS, NEEDS_KINDS, STEWARD_NEED_KINDS, type Binding } from "./store.js"; import { GITHUB_PRS, GITHUB_REVIEWS, LINEAR_TICKETS, OPS_WATCH, DEPS_ALERTS, DEPS_PRS, projectFileTemplate } from "./templates.js"; import { STANDING_RULES } from "./wake.js"; @@ -3597,8 +3597,8 @@ describe("the sidebar's needs you block (BB-46)", () => { const b = t.hooks.store.find("ENG-12", "p1")!; const before = t.harness.inspection.realtimeSignals.length; t.hooks.store.ask({ projectId: "p1", bindingId: b.id, escalationId: null, question: "Which approach?", context: null }); - const signals = t.harness.inspection.realtimeSignals.slice(before).filter((s) => s.channel === NEEDS_YOU_CHANGED); - expect(signals).toEqual([{ channel: NEEDS_YOU_CHANGED, payload: { projectId: "p1" } }]); + const signals = t.harness.inspection.realtimeSignals.slice(before).filter((s) => s.channel === PROJECT_CHANGED); + expect(signals).toEqual([{ channel: PROJECT_CHANGED, payload: { projectId: "p1" } }]); }); it("publishes exactly one event when a question is answered, even when it resolves its escalation too", async () => { @@ -3611,8 +3611,8 @@ describe("the sidebar's needs you block (BB-46)", () => { const before = t.harness.inspection.realtimeSignals.length; const result = await t.harness.callRpc("stewardsAnswerQuestion", { questionId: question.id, answer: "The safe one." }, { experimental_caller: { kind: "client" } }); expect(result).toMatchObject({ ok: true }); - const signals = t.harness.inspection.realtimeSignals.slice(before).filter((s) => s.channel === NEEDS_YOU_CHANGED); - expect(signals).toEqual([{ channel: NEEDS_YOU_CHANGED, payload: { projectId: "p1" } }]); + const signals = t.harness.inspection.realtimeSignals.slice(before).filter((s) => s.channel === PROJECT_CHANGED); + expect(signals).toEqual([{ channel: PROJECT_CHANGED, payload: { projectId: "p1" } }]); }); it("publishes one event when a route-me escalation is filed directly", async () => { @@ -3622,8 +3622,8 @@ describe("the sidebar's needs you block (BB-46)", () => { const b = t.hooks.store.find("ENG-12", "p1")!; const before = t.harness.inspection.realtimeSignals.length; t.hooks.store.escalate({ bindingId: b.id, kind: "decision", question: "Forwarded?", meanwhile: "Wait.", route: "me", recommendation: null, projectId: "p1" }); - const signals = t.harness.inspection.realtimeSignals.slice(before).filter((s) => s.channel === NEEDS_YOU_CHANGED); - expect(signals).toEqual([{ channel: NEEDS_YOU_CHANGED, payload: { projectId: "p1" } }]); + const signals = t.harness.inspection.realtimeSignals.slice(before).filter((s) => s.channel === PROJECT_CHANGED); + expect(signals).toEqual([{ channel: PROJECT_CHANGED, payload: { projectId: "p1" } }]); }); it("publishes when a binding ends and its needs leave the set", async () => { @@ -3634,8 +3634,110 @@ describe("the sidebar's needs you block (BB-46)", () => { t.hooks.store.fileNeed({ bindingId: b.id, kind: "configure", url: "", what: "Enable the flag", how: null }); const before = t.harness.inspection.realtimeSignals.length; t.hooks.store.patch(b.id, { endedAt: t.now(), endReason: "merged" }); - const signals = t.harness.inspection.realtimeSignals.slice(before).filter((s) => s.channel === NEEDS_YOU_CHANGED); - expect(signals).toEqual([{ channel: NEEDS_YOU_CHANGED, payload: { projectId: "p1" } }]); + const signals = t.harness.inspection.realtimeSignals.slice(before).filter((s) => s.channel === PROJECT_CHANGED); + expect(signals).toEqual([{ channel: PROJECT_CHANGED, payload: { projectId: "p1" } }]); + }); +}); + +// BB-90: the channel is a generic "refetch this project" signal, so every +// sidebar- and board-visible binding change publishes on it, not only the +// "needs you" set. +describe("the realtime signal on every sidebar-visible change (BB-90)", () => { + const one = { channel: PROJECT_CHANGED, payload: { projectId: "p1" } }; + const signalsSince = (t: Awaited>, before: number) => t.harness.inspection.realtimeSignals.slice(before).filter((s) => s.channel === PROJECT_CHANGED); + /** A settled steward binding on p1, past the signals its adoption made. */ + async function bound() { + const t = await setup(); + t.entities.set("uuid-12", issue()); + await t.ring("linear", "uuid-12"); + return { t, b: t.hooks.store.find("ENG-12", "p1")! }; + } + + it("publishes one signal when the stage changes", async () => { + const { t, b } = await bound(); + const before = t.harness.inspection.realtimeSignals.length; + t.hooks.store.patch(b.id, { stage: b.stage === "build" ? "review" : "build" }); + expect(signalsSince(t, before)).toEqual([one]); + }); + + it("publishes one signal when a hold is set and another when it is lifted", async () => { + const { t, b } = await bound(); + let before = t.harness.inspection.realtimeSignals.length; + t.hooks.store.patch(b.id, { hold: "waiting on the reviewer" }); + expect(signalsSince(t, before)).toEqual([one]); + before = t.harness.inspection.realtimeSignals.length; + t.hooks.store.patch(b.id, { hold: null }); + expect(signalsSince(t, before)).toEqual([one]); + }); + + it("publishes one signal when a status report is written", async () => { + const { t, b } = await bound(); + const before = t.harness.inspection.realtimeSignals.length; + t.hooks.store.patch(b.id, { + status: { phase: "Build", summary: "Building.", nextAction: "Wait", actor: "steward", blockedOn: null, requeueAfterMs: null, conditions: [], resources: { workers: [], pullRequests: [], children: [] }, at: t.now() }, + advancedAt: t.now(), + }); + expect(signalsSince(t, before)).toEqual([one]); + }); + + it("publishes one signal when a deployment watch changes state", async () => { + const { t, b } = await bound(); + const before = t.harness.inspection.realtimeSignals.length; + t.hooks.store.patch(b.id, { deployments: { "acme/web#7": { key: "acme/web#7", state: "deployed", mergeSha: null, repo: "acme/web", environment: "production", deploymentId: null, deploymentState: null, ref: null, sha: null, url: null, at: t.now(), deadlineAt: null, note: null, workflowRuns: [] } } }); + expect(signalsSince(t, before)).toEqual([one]); + }); + + it("publishes one signal for each of the remaining visible columns", async () => { + const cases: Array<[string, (b: Binding) => Partial]> = [ + ["phase", () => ({ phase: "Building" })], + ["holdBehind", () => ({ holdBehind: ["ENG-9"] })], + ["requeueAt", () => ({ requeueAt: 1_700_000_000_000 })], + ["title", () => ({ title: "A new title" })], + ["url", () => ({ url: "https://linear.app/acme/issue/ENG-12?new" })], + ["ending", () => ({ ending: "terminal" })], + ["threadId", () => ({ threadId: "thr_new" })], + ["pending", (b) => ({ pending: [...b.pending, { at: 1, text: "a queued note" }] })], + ]; + for (const [name, make] of cases) { + const { t, b } = await bound(); + const before = t.harness.inspection.realtimeSignals.length; + t.hooks.store.patch(b.id, make(b)); + expect(signalsSince(t, before), name).toEqual([one]); + } + }); + + it("publishes one signal when a worker is added and another when it finishes", async () => { + const { t, b } = await bound(); + let before = t.harness.inspection.realtimeSignals.length; + t.hooks.store.addWorker("thr_worker", b.id, "Implement"); + expect(signalsSince(t, before)).toEqual([one]); + before = t.harness.inspection.realtimeSignals.length; + t.hooks.store.setWorker("thr_worker", "finished", "done"); + expect(signalsSince(t, before)).toEqual([one]); + before = t.harness.inspection.realtimeSignals.length; + t.hooks.store.setWorker("thr_worker", "finished", "done"); + expect(signalsSince(t, before)).toEqual([]); + }); + + it("stays silent when a note is queued for the next turn", async () => { + const { t, b } = await bound(); + const before = t.harness.inspection.realtimeSignals.length; + t.hooks.store.note(b.id, "A quiet note for the next turn."); + expect(signalsSince(t, before)).toEqual([]); + }); + + it("stays silent when only a non-visible field changes", async () => { + const { t, b } = await bound(); + const before = t.harness.inspection.realtimeSignals.length; + t.hooks.store.patch(b.id, { lastJudgmentAt: t.now() }); + expect(signalsSince(t, before)).toEqual([]); + }); + + it("stays silent when a visible field is set to its current value", async () => { + const { t, b } = await bound(); + const before = t.harness.inspection.realtimeSignals.length; + t.hooks.store.patch(b.id, { stage: b.stage, title: b.title, url: b.url }); + expect(signalsSince(t, before)).toEqual([]); }); }); diff --git a/plugins/stewards/server.ts b/plugins/stewards/server.ts index 8e0cc4e..5fb6cdf 100644 --- a/plugins/stewards/server.ts +++ b/plugins/stewards/server.ts @@ -81,7 +81,7 @@ import { GUIDANCE_UNIT_PREFIX, GUIDANCE_UNCITED_DAYS, escalationWeeks, parseGuid import { createGhWriter, parsePullRef, type GithubWriter } from "./github.js"; import { allPullsMerged, atWork, conflictsOf, finished, normalizeTouches, overlapping, touchesOverlap, type Conflict } from "./schedule.js"; import { AUTOMATIC_THREAD_LIST_PROVIDER, BUILTIN_THREAD_LIST_ID, THREAD_LIST_PROVIDER_KEY } from "./sidebar-provider.js"; -import { NEEDS_YOU_CHANGED } from "./channels.js"; +import { PROJECT_CHANGED } from "./channels.js"; import { PLAYBOOK_TEMPLATES, projectFileTemplate } from "./templates.js"; import type { DeploymentTarget } from "./config.js"; import { PROJECT_STEWARD_INTRO, choicesNote, formatBrief, formatDepsWake, formatOpsWake, formatProjectWake, formatSentryWake, formatSessionWake, formatWake, spawnCommand, workerQuestion, workerReportedError, workerWaiting, workflowRunLine, type DigestRecords, type PendingRequest, type WakeReason, type WorkerQuestion } from "./wake.js"; @@ -735,10 +735,11 @@ export function createStewardsPlugin(deps: StewardsDeps): (bb: BbPluginApi) => P const db = bb.storage.database(); bb.storage.migrate(db, MIGRATIONS); - // The store reports a project whose "needs you" set changed; the sidebar - // and the board page refetch on the channel. Keeping it a callback keeps - // store.ts free of bb, so steward-side mutations publish too. - const store = createStore(db, now, { onNeedsYouChange: (projectId) => bb.realtime.publish(NEEDS_YOU_CHANGED, { projectId }) }); + // The store reports a project whose sidebar- or board-visible state + // changed; the sidebar, the board page and the overview refetch on the + // channel. Keeping it a callback keeps store.ts free of bb, so + // steward-side mutations publish too. + const store = createStore(db, now, { onProjectChange: (projectId) => bb.realtime.publish(PROJECT_CHANGED, { projectId }) }); const rpc = (pluginId: string, method: string, input: unknown, outputSchema: z.ZodType, signal?: AbortSignal): Promise => bb.sdk.plugins.callRpc({ pluginId, method, input: input as never, outputSchema, ...(signal === undefined ? {} : { signal }) }); @@ -4236,19 +4237,24 @@ export function createStewardsPlugin(deps: StewardsDeps): (bb: BbPluginApi) => P if (worker.status === "stopped") return; const result = lastAssistantText === null ? null : clip(lastAssistantText, 4000); store.setWorker(thread.id, "finished", result); + // The quiet-finish note is what the visible "worker finished" phrase + // reads, so write it before the awaits below: `setWorker` has already + // published the signal, and the refetch it triggers would otherwise + // read the store before the note lands (BB-90). + const finished = store.binding(worker.bindingId); + // BB-52 rule 3: a worker that ends its turn waiting on its reviewer, + // the owner or a build is a quiet note. Only a question, an error + // report or a gone thread makes the turn due; a failed turn comes + // through thread.failed and wakes on its own. A binding following + // through is waiting on this worker's result, so its finish is news. + const needsSteward = finished !== null && (workerQuestion(result) !== null || workerReportedError(result) || followingThrough(finished)); + if (finished !== null && finished.endedAt === null) store.note(finished.id, `Worker "${worker.title}" finished its turn.`, !needsSteward); await closeRequesterNeed(thread.id); // Its turn is over: the reviewers it read (or started and then waited // on) have had their round consumed (BB-47). await archiveConsumedReviewers(thread.id); const b = store.binding(worker.bindingId); if (b === null || b.endedAt !== null) return; - // BB-52 rule 3: a worker that ends its turn waiting on its reviewer, - // the owner or a build is a quiet note. Only a question, an error - // report or a gone thread makes the turn due; a failed turn comes - // through thread.failed and wakes on its own. A binding following - // through is waiting on this worker's result, so its finish is news. - const needsSteward = workerQuestion(result) !== null || workerReportedError(result) || followingThrough(b); - store.note(b.id, `Worker "${worker.title}" finished its turn.`, !needsSteward); // A quiet finish is consumed here, not by a wake: archive the worker // now (BB-47), keeping one that still has a question, a reviewer, or is // parked waiting on something that will wake it (BB-52 rule 3). diff --git a/plugins/stewards/store.ts b/plugins/stewards/store.ts index 7a8e712..1c7360a 100644 --- a/plugins/stewards/store.ts +++ b/plugins/stewards/store.ts @@ -723,6 +723,31 @@ const COLUMNS = { export type BindingPatch = Partial>; +/** + * The binding columns the sidebar, the board and the overview show (BB-90). + * `patch` rings the realtime channel only when one of these actually changed + * before and after; a write to `lastJudgmentAt`, `lastSnapshot`, `wokeAt`, + * `awaitingReport`, `missedReports`, `archiveWhenIdle`, `manualDetach`, a + * model-check flag or the like changes nothing a page renders and stays + * silent, as does setting a field to the value it already had. + */ +const VISIBLE_COLUMNS = [ + "stage", + "phase", + "hold", + "hold_behind", + "status", + "pending", + "requeue_at", + "advanced_at", + "deployments", + "title", + "url", + "ending", + "ended_at", + "thread_id", +] as const satisfies ReadonlyArray<(typeof COLUMNS)[keyof typeof COLUMNS]>; + function toColumn(value: unknown): unknown { if (value === undefined || value === null) return null; if (typeof value === "boolean") return value ? 1 : 0; @@ -977,12 +1002,14 @@ export const GUIDANCE_DEDUPE_MS = 5 * 60_000; export const newId = (prefix: string) => `${prefix}_${randomBytes(9).toString("base64url")}`; /** - * What a store reports back to its owner when a project's "needs you" set - * changes. The server wires it to the realtime channel; the store itself - * knows nothing about bb. + * What a store reports back to its owner when a project's sidebar- or + * board-visible state changes: a binding's stage, phase, hold, status report, + * requeue time, deployment state, title, link or ending, a worker starting or + * finishing, or the "needs you" set. The server wires it to the realtime + * channel; the store itself knows nothing about bb. */ export interface StoreOptions { - onNeedsYouChange?: (projectId: string) => void; + onProjectChange?: (projectId: string) => void; } export function createStore(db: Database.Database, now: () => number, options: StoreOptions = {}) { @@ -1306,7 +1333,7 @@ export function createStore(db: Database.Database, now: () => number, options: S resolvedAt: r.resolved_at === null || r.resolved_at === undefined ? null : Number(r.resolved_at), }); - const onNeedsYouChange = options.onNeedsYouChange; + const onProjectChange = options.onProjectChange; /** The project a binding belongs to, for a mutation that only has the binding. */ function projectIdForBinding(bindingId: string): string | null { @@ -1314,24 +1341,25 @@ export function createStore(db: Database.Database, now: () => number, options: S return b === undefined ? null : String(b.project_id); } - // A "needs you" change is coalesced within one synchronous store operation, - // so one question filed or answered publishes exactly one event even when - // the operation nests (answer -> resolve) or closes a migrated need through - // resolve. The flush runs when the outermost operation finishes. + // A project-visible change is coalesced within one synchronous store + // operation, so one question filed or answered publishes exactly one event + // even when the operation nests (answer -> resolve) or closes a migrated + // need through resolve, and a turn that writes several fields publishes one + // signal. The flush runs when the outermost operation finishes. let notifyDepth = 0; const dirtyProjects = new Set(); - function flushNeedsYou(): void { + function flushProjectChanged(): void { const projects = [...dirtyProjects]; dirtyProjects.clear(); - if (onNeedsYouChange !== undefined) for (const projectId of projects) onNeedsYouChange(projectId); + if (onProjectChange !== undefined) for (const projectId of projects) onProjectChange(projectId); } - /** Mark a project's "needs you" set changed; publishes now, or on the way out of a nested operation. */ - function needsYouChanged(projectId: string | null | undefined): void { - if (onNeedsYouChange === undefined || projectId === null || projectId === undefined || projectId === "") return; + /** Mark a project's visible state changed; publishes now, or on the way out of a nested operation. */ + function projectChanged(projectId: string | null | undefined): void { + if (onProjectChange === undefined || projectId === null || projectId === undefined || projectId === "") return; dirtyProjects.add(projectId); - if (notifyDepth === 0) flushNeedsYou(); + if (notifyDepth === 0) flushProjectChanged(); } /** Run `fn`, flushing at most one notification per project when it returns successfully. */ @@ -1340,7 +1368,7 @@ export function createStore(db: Database.Database, now: () => number, options: S try { const result = fn(); notifyDepth -= 1; - if (notifyDepth === 0) flushNeedsYou(); + if (notifyDepth === 0) flushProjectChanged(); return result; } catch (error) { // A failed operation did not change anything: drop its pending marks. @@ -1379,7 +1407,7 @@ export function createStore(db: Database.Database, now: () => number, options: S })(); if (resolved) { const x = getEscalation.get(id); - needsYouChanged(x === undefined ? null : projectIdForBinding(String(x.binding_id))); + projectChanged(x === undefined ? null : projectIdForBinding(String(x.binding_id))); } return resolved; }); @@ -1417,17 +1445,26 @@ export function createStore(db: Database.Database, now: () => number, options: S if (deliver && x.origin.threadId !== null && x.origin.threadId !== "") insertDelivery.run(newId("dlv"), x.id, x.origin.threadId, originBindingId === "" ? null : originBindingId, resolution, by, now()); } - function patch(id: string, fields: BindingPatch): Binding { - const wasOpen = getBinding.get(id)?.ended_at === null; + /** Write the given columns and return the raw row, with no realtime signal. */ + function writeBinding(id: string, fields: BindingPatch): BindingRow { const entries = Object.entries(fields).filter(([, v]) => v !== undefined) as Array<[keyof typeof COLUMNS, unknown]>; if (entries.length > 0) { const sets = entries.map(([k]) => `${COLUMNS[k]} = ?`).join(", "); db.prepare(`UPDATE bindings SET ${sets} WHERE id = ?`).run(...entries.map(([, v]) => toColumn(v)), id); } - const b = rowToBinding(getBinding.get(id)!); - // Ending a binding takes its needs and with-user escalations off the board - // and the sidebar; reopening one puts them back. - if (fields.endedAt !== undefined && wasOpen !== (b.endedAt === null)) needsYouChanged(b.projectId); + return getBinding.get(id)!; + } + + function patch(id: string, fields: BindingPatch): Binding { + const before = getBinding.get(id); + const after = writeBinding(id, fields); + const b = rowToBinding(after); + // A field the sidebar or board shows changing rings the refetch channel; + // per-turn bookkeeping (lastJudgmentAt, lastSnapshot, wokeAt, …) and a + // same-value write change nothing visible and stay silent. Ending or + // reopening a binding is covered here too: endedAt is one of these fields, + // so its needs and with-user escalations leave or return with the signal. + if (before !== undefined && VISIBLE_COLUMNS.some((c) => before[c] !== after[c])) projectChanged(b.projectId); return b; } @@ -1495,7 +1532,13 @@ export function createStore(db: Database.Database, now: () => number, options: S const i = pending.findIndex((n) => n.quiet === true); pending.splice(i === -1 ? 0 : i, 1); } - return patch(id, { pending }); + // A queued note is a reason for the next judgment turn, not a page + // change, so it writes without a signal (BB-90). Its one visible effect, + // the "worker finished" phrase, is published by `setWorker` when the + // worker's status changes; ringing here would add a second signal for + // that finish and a second event in the answer path, where the note is + // written after `answer`'s operation has already flushed. + return rowToBinding(writeBinding(id, { pending })); }, /** Save guidance; false when it repeats the binding's last guidance by the same author within five minutes (a retry), which saves nothing. */ addGuidance(id: string, text: string, by: string): boolean { @@ -1517,7 +1560,7 @@ export function createStore(db: Database.Database, now: () => number, options: S patch(bindingId, { ready: { ...b.ready, approval, voidedAt: null, declinedSpecHash: null }, declinedSpecObjections: [] }); closeNeedKindStmt.run("Approved.", now(), bindingId, "approve-spec"); for (const x of openEscalations.all(bindingId).map(toEscalation).filter((e) => e.kind === "approval" && e.resolutionKind !== "approve-decline")) resolve(x.id, resolution, by); - needsYouChanged(projectIdForBinding(bindingId)); + projectChanged(projectIdForBinding(bindingId)); })(), ); }, @@ -1529,7 +1572,7 @@ export function createStore(db: Database.Database, now: () => number, options: S patch(bindingId, { ready: { ...b.ready, declinedSpecHash: specHash } }); closeNeedKindStmt.run("Declined.", now(), bindingId, "approve-spec"); for (const x of openEscalations.all(bindingId).map(toEscalation).filter((e) => e.kind === "approval" && e.resolutionKind !== "approve-decline")) resolve(x.id, resolution, by); - needsYouChanged(projectIdForBinding(bindingId)); + projectChanged(projectIdForBinding(bindingId)); })(), ); }, @@ -1576,7 +1619,7 @@ export function createStore(db: Database.Database, now: () => number, options: S head_sha: fields.headSha ?? null, }); // A route-`me` escalation is a "needs you" item the moment it is filed. - if (fields.route === "me") needsYouChanged(fields.projectId ?? projectIdForBinding(fields.bindingId)); + if (fields.route === "me") projectChanged(fields.projectId ?? projectIdForBinding(fields.bindingId)); return toEscalation(getEscalation.get(id)!); }, escalation(id: string): Escalation | null { @@ -1625,7 +1668,7 @@ export function createStore(db: Database.Database, now: () => number, options: S // A route-`me` escalation is a "needs you" item: forwarding one to the // user adds it, and moving one off the user removes it. A reroute to the // route it already had changes nothing on the board. - if (changed && before !== undefined && String(before.route) !== route && (String(before.route) === "me" || route === "me")) needsYouChanged(projectIdForBinding(String(before.binding_id))); + if (changed && before !== undefined && String(before.route) !== route && (String(before.route) === "me" || route === "me")) projectChanged(projectIdForBinding(String(before.binding_id))); return changed; }, /** Re-point a pull-request approval escalation at a new head sha (a push voids the old approval). */ @@ -1705,7 +1748,7 @@ export function createStore(db: Database.Database, now: () => number, options: S data: JSON.stringify(fields.data ?? {}), origin: JSON.stringify({ threadId: null, bindingId: fields.bindingId, chain: [] }), }); - needsYouChanged(fields.projectId); + projectChanged(fields.projectId); return toQuestion(getQuestion.get(id)!); }, question(id: string): Question | null { @@ -1735,7 +1778,7 @@ export function createStore(db: Database.Database, now: () => number, options: S if (answerQuestion.run(answer, by, at, id).changes === 0) return false; const q = toQuestion(getQuestion.get(id)!); if (q.escalationId !== null) resolve(q.escalationId, resolution, by); - needsYouChanged(q.projectId); + projectChanged(q.projectId); return true; })(), ); @@ -1750,7 +1793,7 @@ export function createStore(db: Database.Database, now: () => number, options: S const changed = answerQuestion.run(answer, by, now(), id).changes > 0; if (changed) { const q = getQuestion.get(id); - needsYouChanged(q === undefined || q.project_id === null || q.project_id === undefined ? null : String(q.project_id)); + projectChanged(q === undefined || q.project_id === null || q.project_id === undefined ? null : String(q.project_id)); } return changed; }); @@ -1791,19 +1834,19 @@ export function createStore(db: Database.Database, now: () => number, options: S const existing = openNeedByKey.get(fields.bindingId, fields.kind, fields.url); if (existing !== undefined) { updateNeed.run(fields.what, fields.how, JSON.stringify(fields.data ?? {}), existing.id); - needsYouChanged(projectId); + projectChanged(projectId); return toNeed(getNeed.get(String(existing.id))!); } const id = newId("need"); insertNeed.run({ id, binding_id: fields.bindingId, kind: fields.kind, url: fields.url, what: fields.what, how: fields.how, data: JSON.stringify(fields.data ?? {}), filed_by: fields.filedBy ?? "steward", created_at: now() }); - needsYouChanged(projectId); + projectChanged(projectId); return toNeed(getNeed.get(id)!); }, closeNeed(id: string, resolution: string, by = "plugin"): boolean { return coalescing(() => { const row = getNeed.get(id); if (closeNeedStmt.run(resolution, now(), id).changes > 0) { - needsYouChanged(row === undefined ? null : projectIdForBinding(String(row.binding_id))); + projectChanged(row === undefined ? null : projectIdForBinding(String(row.binding_id))); return true; } // A migrated provide-secret need is an escalation now. @@ -1812,18 +1855,20 @@ export function createStore(db: Database.Database, now: () => number, options: S }, closeNeeds(bindingId: string, kind: NeedKind, resolution: string): number { const changed = closeNeedKindStmt.run(resolution, now(), bindingId, kind).changes; - if (changed > 0) needsYouChanged(projectIdForBinding(bindingId)); + if (changed > 0) projectChanged(projectIdForBinding(bindingId)); return changed; }, closeNeedByKey(bindingId: string, kind: NeedKind, url: string, resolution: string): number { const changed = closeNeedKeyStmt.run(resolution, now(), bindingId, kind, url).changes; - if (changed > 0) needsYouChanged(projectIdForBinding(bindingId)); + if (changed > 0) projectChanged(projectIdForBinding(bindingId)); return changed; }, // workers addWorker(threadId: string, bindingId: string, title: string): void { insertWorker.run(threadId, bindingId, title, now()); + // A worker starting moves its row at once, like any visible field. + projectChanged(projectIdForBinding(bindingId)); }, worker(threadId: string): Worker | null { const r = getWorker.get(threadId); @@ -1836,7 +1881,12 @@ export function createStore(db: Database.Database, now: () => number, options: S return spawnsSince.get(bindingId, since)!.n; }, setWorker(threadId: string, status: WorkerStatus, result: string | null): void { + const before = getWorker.get(threadId); setWorker.run(status, result, status === "running" ? null : now(), threadId); + const w = getWorker.get(threadId); + // A worker finishing or being stopped moves its row at once, but a repeat + // write of the same status changes nothing visible and stays silent. + if (w !== undefined && (before === undefined || before.status !== w.status)) projectChanged(projectIdForBinding(String(w.binding_id))); }, /** Whether the worker's model is still to be checked when its first turn is dispatched. */ setWorkerModelCheckPending(threadId: string, pending: boolean): void {