diff --git a/plugins/stewards/app.tsx b/plugins/stewards/app.tsx index bbb17cf..993f93c 100644 --- a/plugins/stewards/app.tsx +++ b/plugins/stewards/app.tsx @@ -33,7 +33,7 @@ import { } from "@get-bb/plugin-sdk/app"; import type { rpcContract } from "./server"; 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 { buildSidebar, compareBlocked, compareEfforts, compareInflight, countThreads, deploymentPhrase, effortPhrase, effortSection, effortState, effortTitle, hasUnread, isBusy, isUnread, 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"; type Client = ReturnType>; @@ -1204,10 +1204,6 @@ function actionTarget(t: PluginSidebarThread): PluginThreadActionTarget { }; } -function countThreads(children: readonly SidebarChild[]): number { - return children.reduce((n, c) => n + 1 + countThreads(c.children), 0); -} - /** A pull-request key like `acme/web#612` shortens to `#612`; a ticket key stays its key. */ function shortBindingKey(key: string): string { const pull = /#(\d+)$/.exec(key); @@ -1261,7 +1257,7 @@ function StewardGlyph({ kind }: { kind: SidebarEffortState }) { /** A one-off thread's glyph: yellow when unread, blue animated while its agent runs, grey otherwise. */ function oneOffGlyph(thread: PluginSidebarThread): SidebarEffortState { - if (thread.isUnread || thread.indicator === "unread-success" || thread.indicator === "unread-error") return "you"; + if (isUnread(thread)) return "you"; if (isBusy(thread.status)) return "running"; return "quiet"; } @@ -1305,13 +1301,16 @@ function SidebarThreadRow({ const navigate = useBbNavigate(); const rowStatus = useSidebarThreadRowStatus(thread.id); const draft = useSidebarThreadDraft(thread.id); - const [open, setOpen] = useState(false); + const [userOpen, setUserOpen] = useState(null); const selected = activeThreadId !== null && thread.id === activeThreadId; // Navigation to a row inside this one expands it, so the highlighted row is revealed. const revealed = openThreads.has(thread.id); useEffect(() => { - if (revealed) setOpen(true); + if (revealed) setUserOpen(true); }, [revealed]); + // An untouched row follows the unread state of its descendants (BB-80); a + // manual toggle always wins. + const open = userOpen ?? hasUnread(children, byId); // The array-wide indicator never reports a draft; the row folds in the // client-local one, as bb's own row does. const indicator = draft.hasUnsubmittedDraft && thread.indicator === "none" ? (isBusy(thread.status) ? "working-draft" : "draft") : thread.indicator; @@ -1327,7 +1326,7 @@ function SidebarThreadRow({ aria-label={`${open ? "Collapse" : "Expand"} ${thread.displayTitle}`} aria-expanded={open} className="flex size-4 shrink-0 items-center justify-center text-muted-foreground" - onClick={() => setOpen((v) => !v)} + onClick={() => setUserOpen(!open)} > @@ -1483,7 +1482,7 @@ function SidebarEffortRow({ const root = byId.get(effort.rootThreadId); const navigate = useBbNavigate(); const pluginId = experimental_usePluginId(); - const [open, setOpen] = useState(false); + const [userOpen, setUserOpen] = useState(null); const key = effort.bindingKey; if (key === null) return null; const displayKey = shortBindingKey(key); @@ -1496,8 +1495,11 @@ function SidebarEffortRow({ const selected = activeKey !== null && activeKey === key; const revealed = openThreads.has(effort.rootThreadId); useEffect(() => { - if (revealed) setOpen(true); + if (revealed) setUserOpen(true); }, [revealed]); + // An untouched effort follows the unread state of its workers and reviewers + // (BB-80); a manual toggle always wins. + const open = userOpen ?? hasUnread(effort.children, byId); const href = boardHref(pluginId, projectId, key); const body = (
@@ -1508,7 +1510,7 @@ function SidebarEffortRow({ aria-label={`${open ? "Collapse" : "Expand"} ${root?.displayTitle ?? effort.title}`} aria-expanded={open} className="flex size-4 shrink-0 items-center justify-center text-muted-foreground" - onClick={() => setOpen((v) => !v)} + onClick={() => setUserOpen(!open)} > @@ -1579,12 +1581,15 @@ function SidebarSection({ title, count, children }: { title: string; count: numb ); } -function SidebarCollapsible({ title, count, revealed = false, children }: { title: string; count: number; revealed?: boolean; children: ReactNode }) { - const [open, setOpen] = useState(false); +function SidebarCollapsible({ title, count, revealed = false, unread = false, children }: { title: string; count: number; revealed?: boolean; /** Whether a thread in the section is unread, so an untouched section opens (BB-80). */ unread?: boolean; children: ReactNode }) { + // An untouched section follows the unread state of the threads in it + // (BB-80); a manual toggle always wins. + const [userOpen, setUserOpen] = useState(null); // Navigation to a row inside this section expands it, so the row is revealed. useEffect(() => { - if (revealed) setOpen(true); + if (revealed) setUserOpen(true); }, [revealed]); + const open = userOpen ?? unread; return (
{/* `
` only for its `open` attribute, which drives the chevron's @@ -1597,7 +1602,7 @@ function SidebarCollapsible({ title, count, revealed = false, children }: { titl className="flex cursor-pointer list-none items-center gap-1 px-2 py-1 text-xs font-medium uppercase tracking-wide text-muted-foreground hover:text-foreground" onClick={(e) => { e.preventDefault(); - setOpen((v) => !v); + setUserOpen(!open); }} > @@ -1662,6 +1667,14 @@ function subtreeHas(children: readonly SidebarChild[], id: string): boolean { return children.some((c) => c.threadId === id || subtreeHas(c.children, id)); } +/** Whether any effort in a section has an unread root or descendant thread (BB-80). */ +function sectionHasUnread(efforts: readonly SidebarEffort[], byId: ReadonlyMap): boolean { + return efforts.some((e) => { + const root = byId.get(e.rootThreadId); + return (root !== undefined && isUnread(root)) || hasUnread(e.children, byId); + }); +} + /** The ancestor thread ids from the direct parent up, or null when `id` is not in the tree. */ function ancestorThreads(children: readonly SidebarChild[], id: string): string[] | null { for (const c of children) { @@ -1809,7 +1822,7 @@ function StewardsSidebarCore({ )} {active.threads.length > 0 ? ( - +
    {active.threads.map((e) => { const t = byId.get(e.rootThreadId); @@ -1821,12 +1834,12 @@ function StewardsSidebarCore({ ) : null} {active.monitors.length > 0 ? ( - +
      {rows(active.monitors)}
     ) : null} {active.ended.length > 0 ? ( - +
      {active.ended.map((e) => ( { } as never, }); // An unread worker is the yellow dot even with no running worker; its busy - // reviewer is the pulsing blue ring. - fireEvent.click(await waitFor(() => slot.getByRole("button", { name: /Expand ENG-1: Fix login/u }))); + // reviewer is the pulsing blue ring. BB-80: the unread worker opens the + // effort by default, so there is no Expand click here. await waitFor(() => expect(slot.getByText("Unread worker")).toBeTruthy()); + expect(slot.getByRole("button", { name: /Collapse ENG-1: Fix login/u })).toBeTruthy(); expect(glyphOf(slot, "Unread worker")).toBe("you"); fireEvent.click(slot.getByRole("button", { name: /Expand Unread worker/u })); await waitFor(() => expect(slot.getByText("Busy reviewer")).toBeTruthy()); @@ -696,7 +697,8 @@ describe("the stewards sidebar sections, glyphs and phrases (BB-76)", () => { }, rpc: { stewardsSidebar: () => sidebarData() } as never, }); - fireEvent.click(await waitFor(() => slot.getByRole("button", { name: /Threads/u }))); + // BB-80: the unread one-off opens the Threads section by default. + await waitFor(() => expect(slot.getByRole("button", { name: /Threads/u }).closest("section")!.querySelector("[data-sidebar-collapsible]")!.className).toContain("grid-rows-[1fr]")); await waitFor(() => expect(slot.getByText("Unread reply")).toBeTruthy()); expect(glyphOf(slot, "Unread reply")).toBe("you"); expect(glyphOf(slot, "Running agent")).toBe("running"); @@ -1172,3 +1174,160 @@ describe("the sidebar's row transitions (BB-91)", () => { slot.lifecycle.unmount(); }); }); + +// BB-80: a group with an unread descendant starts expanded; a manual toggle +// always wins, and an untouched group follows unread state as it changes. +describe("the sidebar's unread auto-expand (BB-80)", () => { + const workersBody = (slot: RenderedSlot, title: string): HTMLElement => slot.getByText(title).closest("li")!.querySelector("[data-sidebar-workers]")!; + const sectionBody = (slot: RenderedSlot, name: RegExp): HTMLElement => slot.getByRole("button", { name }).closest("section")!.querySelector("[data-sidebar-collapsible]")!; + + it("expands a one-off thread row and the Threads section when a descendant is unread on first render", async () => { + const slot = await renderSidebar({ + sidebarThreads: { + projects, + threads: [thread({ id: "thr_parent", displayTitle: "One-off parent" }), thread({ id: "thr_child", displayTitle: "Unread child", parentThreadId: "thr_parent", isUnread: true })], + }, + rpc: { stewardsSidebar: () => sidebarData() } as never, + }); + await waitFor(() => expect(sectionBody(slot, /Threads/u).className).toContain("grid-rows-[1fr]")); + // The parent row is open, so its child row is mounted. + await waitFor(() => expect(slot.getByText("Unread child")).toBeTruthy()); + expect(slot.getByRole("button", { name: "Collapse One-off parent" })).toBeTruthy(); + slot.lifecycle.unmount(); + }); + + it("expands an effort row and the Monitors section when a descendant is unread on first render", async () => { + const slot = await renderSidebar({ + sidebarThreads: { + projects, + threads: [thread({ id: "thr_ops", displayTitle: "Ops watch: acme/web" }), thread({ id: "thr_ops_worker", displayTitle: "Check Sentry", isUnread: true })], + }, + rpc: { + stewardsSidebar: () => + sidebarData({ bindings: [binding({ key: "acme/web", kind: "ops", stage: null, threadId: "thr_ops", workers: [{ threadId: "thr_ops_worker", title: "Check Sentry", status: "finished" }] })] }), + } as never, + }); + await waitFor(() => expect(sectionBody(slot, /Monitors/u).className).toContain("grid-rows-[1fr]")); + await waitFor(() => expect(slot.getByRole("button", { name: "Collapse Ops watch: acme/web" })).toBeTruthy()); + expect(workersBody(slot, "Ops watch: acme/web").className).toContain("grid-rows-[1fr]"); + slot.lifecycle.unmount(); + }); + + it("leaves all-read groups collapsed on first render", async () => { + const slot = await renderSidebar({ + sidebarThreads: { + projects, + threads: [thread({ id: "thr_steward", displayTitle: "ENG-1: Fix login" }), thread({ id: "thr_worker", displayTitle: "Read worker" }), thread({ id: "thr_parent", displayTitle: "One-off parent" }), thread({ id: "thr_child", displayTitle: "Read child", parentThreadId: "thr_parent" })], + }, + rpc: { stewardsSidebar: () => sidebarData({ bindings: [binding({ workers: [{ threadId: "thr_worker", title: "Read worker", status: "finished" }] })] }) } as never, + }); + await waitFor(() => expect(slot.getByText("Fix login")).toBeTruthy()); + expect(slot.getByRole("button", { name: "Expand ENG-1: Fix login" })).toBeTruthy(); + expect(workersBody(slot, "Fix login").className).toContain("grid-rows-[0fr]"); + expect(sectionBody(slot, /Threads/u).className).toContain("grid-rows-[0fr]"); + expect(slot.queryByText("Read child")).toBeNull(); + slot.lifecycle.unmount(); + }); + + it("flips an untouched group with unread state: expands when unread arrives, collapses when it clears", async () => { + const worker = thread({ id: "thr_worker", displayTitle: "Worker" }); + let calls = 0; + const slot = await renderSidebar({ + sidebarThreads: { projects, threads: [thread({ id: "thr_steward", displayTitle: "ENG-1: Fix login" }), worker] }, + rpc: { + stewardsSidebar: () => { + calls += 1; + return sidebarData({ bindings: [binding({ workers: [{ threadId: "thr_worker", title: "Worker", status: "finished" }] })] }); + }, + } as never, + }); + await waitFor(() => expect(slot.getByText("Fix login")).toBeTruthy()); + expect(slot.getByRole("button", { name: "Expand ENG-1: Fix login" })).toBeTruthy(); + const before = calls; + worker.isUnread = true; + await slot.emitRealtime(PROJECT_CHANGED, { projectId: "p1" }); + await waitFor(() => expect(calls).toBeGreaterThan(before)); + await waitFor(() => expect(slot.getByRole("button", { name: "Collapse ENG-1: Fix login" })).toBeTruthy()); + const afterExpand = calls; + worker.isUnread = false; + await slot.emitRealtime(PROJECT_CHANGED, { projectId: "p1" }); + await waitFor(() => expect(calls).toBeGreaterThan(afterExpand)); + await waitFor(() => expect(slot.getByRole("button", { name: "Expand ENG-1: Fix login" })).toBeTruthy()); + slot.lifecycle.unmount(); + }); + + it("keeps a manually collapsed group collapsed when new unread arrives", async () => { + const first = thread({ id: "thr_w1", displayTitle: "Unread worker", isUnread: true }); + const second = thread({ id: "thr_w2", displayTitle: "Second worker" }); + let calls = 0; + const slot = await renderSidebar({ + sidebarThreads: { projects, threads: [thread({ id: "thr_steward", displayTitle: "ENG-1: Fix login" }), first, second] }, + rpc: { + stewardsSidebar: () => { + calls += 1; + return sidebarData({ bindings: [binding({ workers: [{ threadId: "thr_w1", title: "Unread worker", status: "finished" }, { threadId: "thr_w2", title: "Second worker", status: "finished" }] })] }); + }, + } as never, + }); + // Auto-expanded by the first worker's unread, then collapsed by hand. + fireEvent.click(await waitFor(() => slot.getByRole("button", { name: "Collapse ENG-1: Fix login" }))); + await waitFor(() => expect(slot.getByRole("button", { name: "Expand ENG-1: Fix login" })).toBeTruthy()); + // New unread arrives on the second worker; the manual collapse stands. + const before = calls; + second.isUnread = true; + await slot.emitRealtime(PROJECT_CHANGED, { projectId: "p1" }); + await waitFor(() => expect(calls).toBeGreaterThan(before)); + await waitFor(() => expect(slot.getByRole("button", { name: "Expand ENG-1: Fix login" })).toBeTruthy()); + slot.lifecycle.unmount(); + }); + + it("keeps a manually expanded group expanded after everything becomes read", async () => { + const worker = thread({ id: "thr_worker", displayTitle: "Worker" }); + let calls = 0; + const slot = await renderSidebar({ + sidebarThreads: { projects, threads: [thread({ id: "thr_steward", displayTitle: "ENG-1: Fix login" }), worker] }, + rpc: { + stewardsSidebar: () => { + calls += 1; + return sidebarData({ bindings: [binding({ workers: [{ threadId: "thr_worker", title: "Worker", status: "finished" }] })] }); + }, + } as never, + }); + // All read: collapsed, then expanded by hand. + fireEvent.click(await waitFor(() => slot.getByRole("button", { name: "Expand ENG-1: Fix login" }))); + await waitFor(() => expect(slot.getByRole("button", { name: "Collapse ENG-1: Fix login" })).toBeTruthy()); + // Unread arrives and clears again; the manual expand stands. + const beforeUnread = calls; + worker.isUnread = true; + await slot.emitRealtime(PROJECT_CHANGED, { projectId: "p1" }); + await waitFor(() => expect(calls).toBeGreaterThan(beforeUnread)); + await waitFor(() => expect(slot.getByRole("button", { name: "Collapse ENG-1: Fix login" })).toBeTruthy()); + const beforeRead = calls; + worker.isUnread = false; + await slot.emitRealtime(PROJECT_CHANGED, { projectId: "p1" }); + await waitFor(() => expect(calls).toBeGreaterThan(beforeRead)); + await waitFor(() => expect(slot.getByRole("button", { name: "Collapse ENG-1: Fix login" })).toBeTruthy()); + slot.lifecycle.unmount(); + }); + + // BB-85 kept: navigation to a thread inside a group re-expands it even after + // a manual collapse, so the highlighted row is revealed. + it("re-expands a manually collapsed effort when navigation later targets a worker inside it", async () => { + const captured = await loadApp(); + const list = captured.threadLists[0]!; + const props = { activeThreadId: null, activeProjectId: "p1", isCompactViewport: false, onNavigate: () => {}, searchQuery: "" }; + const slot = renderSlot(list, props, { + sidebarThreads: { projects, threads: [thread({ id: "thr_steward", displayTitle: "ENG-1: Fix login" }), thread({ id: "thr_worker", displayTitle: "Worker" })] }, + rpc: { stewardsSidebar: () => sidebarData({ bindings: [binding({ workers: [{ threadId: "thr_worker", title: "Worker", status: "finished" }] })] }) } as never, + }); + // All read: collapsed, expanded and collapsed by hand. + fireEvent.click(await waitFor(() => slot.getByRole("button", { name: "Expand ENG-1: Fix login" }))); + await waitFor(() => expect(slot.getByRole("button", { name: "Collapse ENG-1: Fix login" })).toBeTruthy()); + fireEvent.click(slot.getByRole("button", { name: "Collapse ENG-1: Fix login" })); + await waitFor(() => expect(slot.getByRole("button", { name: "Expand ENG-1: Fix login" })).toBeTruthy()); + // Navigation now targets the worker inside, so the effort re-expands. + slot.lifecycle.rerender(); + await waitFor(() => expect(slot.getByRole("button", { name: "Collapse ENG-1: Fix login" })).toBeTruthy()); + slot.lifecycle.unmount(); + }); +}); diff --git a/plugins/stewards/sidebar.test.ts b/plugins/stewards/sidebar.test.ts index a424877..45743d3 100644 --- a/plugins/stewards/sidebar.test.ts +++ b/plugins/stewards/sidebar.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "vitest"; -import { buildSidebar, compareBlocked, compareEfforts, compareInflight, deploymentPhrase, effortPhrase, effortSection, effortState, effortTitle, endReasonPhrase, endedEffort, isBusy, type SidebarEffort, type SidebarLiveProject, type SidebarLiveThread, type SidebarProjectData } from "./sidebar.js"; +import { buildSidebar, compareBlocked, compareEfforts, compareInflight, deploymentPhrase, effortPhrase, effortSection, effortState, effortTitle, endReasonPhrase, endedEffort, hasUnread, isBusy, isUnread, type SidebarChild, type SidebarEffort, type SidebarLiveProject, type SidebarLiveThread, type SidebarProjectData } from "./sidebar.js"; const thread = (over: Partial & { id: string }): SidebarLiveThread => ({ projectId: "p1", @@ -473,3 +473,33 @@ describe("stewards sidebar Recently ended efforts (BB-88)", () => { expect(tabs[0]!.ended.map((e) => `${e.key}:${e.kind}:${e.phase}:${e.phrase}:${e.state}:${e.endedAt}`)).toEqual(["ENG-0:ticket:Done:terminal:quiet:1000"]); }); }); + +// BB-80: the unread predicate and the descendant walk that decide a group's +// default expansion. +describe("stewards sidebar unread (BB-80)", () => { + const child = (threadId: string, children: SidebarChild[] = []): SidebarChild => ({ threadId, title: threadId, role: "thread", children }); + const flags = (entries: Array<[string, Partial<{ isUnread: boolean; indicator: string }>]>): ReadonlyMap => + new Map(entries.map(([id, over]) => [id, { isUnread: false, indicator: "none", ...over }] as const)); + + it("treats isUnread and both unread indicators as unread, and nothing else", () => { + expect(isUnread({ isUnread: true, indicator: "none" })).toBe(true); + expect(isUnread({ isUnread: false, indicator: "unread-success" })).toBe(true); + expect(isUnread({ isUnread: false, indicator: "unread-error" })).toBe(true); + expect(isUnread({ isUnread: false, indicator: "draft" })).toBe(false); + expect(isUnread({ isUnread: false, indicator: "none" })).toBe(false); + }); + + it("finds an unread thread at any depth", () => { + const tree = [child("a", [child("b", [child("c")])])]; + expect(hasUnread(tree, flags([["a", {}]]))).toBe(false); + expect(hasUnread(tree, flags([["b", { isUnread: true }]]))).toBe(true); + expect(hasUnread(tree, flags([["c", { indicator: "unread-success" }]]))).toBe(true); + expect(hasUnread(tree, flags([["c", { indicator: "unread-error" }]]))).toBe(true); + }); + + it("is false for an empty tree, all-read threads, and threads missing from byId", () => { + expect(hasUnread([], flags([]))).toBe(false); + expect(hasUnread([child("a"), child("b")], flags([["a", {}], ["b", { indicator: "draft" }]]))).toBe(false); + expect(hasUnread([child("a", [child("b")])], flags([]))).toBe(false); + }); +}); diff --git a/plugins/stewards/sidebar.ts b/plugins/stewards/sidebar.ts index 64fddb7..1edbd7f 100644 --- a/plugins/stewards/sidebar.ts +++ b/plugins/stewards/sidebar.ts @@ -175,6 +175,36 @@ export const isBusy = (status: string): boolean => BUSY.has(status); const parentOf = (t: Pick): string | null => t.parentThreadId ?? t.lifecycleOwnerThreadId; +/** Count every thread in a child tree, at any depth. */ +export function countThreads(children: readonly SidebarChild[]): number { + return children.reduce((n, c) => n + 1 + countThreads(c.children), 0); +} + +/** The fields the unread test reads (BB-80): the same pair the sidebar's glyph reads. */ +export interface UnreadFlags { + isUnread: boolean; + indicator: string; +} + +/** + * Whether a thread is unread (BB-80): `isUnread`, or an `unread-success` / + * `unread-error` indicator. The one predicate the glyph and the group defaults + * share. + */ +export const isUnread = (t: UnreadFlags): boolean => t.isUnread || t.indicator === "unread-success" || t.indicator === "unread-error"; + +/** + * Whether any thread in `children` is unread, at any depth (BB-80). A thread + * missing from `byId` counts as read, so an ended or archived row never opens + * a group by itself. + */ +export function hasUnread(children: readonly SidebarChild[], byId: ReadonlyMap): boolean { + return children.some((c) => { + const t = byId.get(c.threadId); + return (t !== undefined && isUnread(t)) || hasUnread(c.children, byId); + }); +} + /** * The fields the glyph, section and phrase rules read. A `SidebarEffort` has * all of them; the Stewards page builds one from its board RPC's steward shape,