From 302368243475b0f5d192e79a51431feacfae24a3 Mon Sep 17 00:00:00 2001 From: Dylan Trotter Date: Wed, 28 Oct 2026 20:07:35 +0000 Subject: [PATCH 1/2] stewards: open the sidebar's PR from the detail page's pane (BB-102) --- plugins/stewards/app.tsx | 92 +++++++++++++++++++++++-- plugins/stewards/sidebar-app.test.tsx | 96 ++++++++++++++++++++++++++- 2 files changed, 183 insertions(+), 5 deletions(-) diff --git a/plugins/stewards/app.tsx b/plugins/stewards/app.tsx index 6b9d563..31c1592 100644 --- a/plugins/stewards/app.tsx +++ b/plugins/stewards/app.tsx @@ -10,7 +10,7 @@ // item's button. Settings → Plugins → Stewards: which projects are stewarded // and whether their configuration is healthy, then every open steward and // where it stands. Steering stays with the project steward and the CLI. -import { memo, useCallback, useEffect, useMemo, useRef, useState, type ReactNode } from "react"; +import { memo, useCallback, useEffect, useMemo, useRef, useState, useSyncExternalStore, type ReactNode } from "react"; import { Markdown, ThreadTitle, @@ -52,11 +52,63 @@ const BOARD_PATH = "board"; const POLL_MS = 30_000; /** How long a burst of refetch requests is coalesced into one RPC (BB-95). */ const REFETCH_DEBOUNCE_MS = 300; +/** How long the sidebar's Open PR handoff waits for the detail page before opening externally (BB-102). */ +const OPEN_PR_HANDOFF_MS = 500; /** Stable empties, so a memoised row's props keep their identity (BB-95). */ const EMPTY_NEEDS: readonly SidebarNeedItem[] = []; const NO_CHILDREN: readonly SidebarChild[] = []; +// A one-shot handoff from the sidebar's Open PR button to the effort detail +// page (BB-102). The sidebar is mounted outside the route content, so its +// `openUrl` never reaches the in-app browser; the detail page sits inside a +// pane whose `openUrl` does. The URL is carried in memory, not in the route, so +// back, forward and a refresh do not replay the open. +type OpenPrHandoff = { url: string; seq: number }; +let openPrHandoff: OpenPrHandoff | null = null; +let openPrSeq = 0; +const openPrListeners = new Set<() => void>(); + +/** Record the URL for the page to open; the returned sequence is the consumer's ticket. */ +function setOpenPrHandoff(url: string): number { + openPrSeq += 1; + openPrHandoff = { url, seq: openPrSeq }; + for (const listener of openPrListeners) listener(); + return openPrSeq; +} + +/** Take the URL once, only while `seq` is still the slot's; a stale or repeated call gets null. */ +function takeOpenPrHandoff(seq: number): string | null { + if (openPrHandoff === null || openPrHandoff.seq !== seq) return null; + const { url } = openPrHandoff; + openPrHandoff = null; + for (const listener of openPrListeners) listener(); + return url; +} + +function subscribeOpenPr(listener: () => void): () => void { + openPrListeners.add(listener); + return () => { + openPrListeners.delete(listener); + }; +} + +function getOpenPrHandoff(): OpenPrHandoff | null { + return openPrHandoff; +} + +/** The pending Open PR handoff, so the page re-renders when one arrives. */ +function useOpenPrHandoff(): OpenPrHandoff | null { + return useSyncExternalStore(subscribeOpenPr, getOpenPrHandoff, getOpenPrHandoff); +} + +/** Drop any pending handoff; tests use it so one case cannot leak into the next. */ +export function resetOpenPrHandoff(): void { + if (openPrHandoff === null) return; + openPrHandoff = null; + for (const listener of openPrListeners) listener(); +} + function message(error: unknown): string { return error instanceof Error ? error.message : String(error); } @@ -808,6 +860,17 @@ function LessonsBlock({ lessons, counts, now }: { lessons: Board["lessons"]; cou function EffortDetail({ board: b, effortKey, refetch }: { board: Board; effortKey: string; refetch: () => void }) { const now = Date.now(); const navigate = useBbNavigate(); + // BB-102: the sidebar's Open PR button leaves the PR URL in the one-shot + // handoff; this page is inside a pane, so its `openUrl` reaches the in-app + // browser. One effect keyed on the sequence opens it and clears the slot, so + // an unmount/remount (or a back/forward) does not replay it. + const handoff = useOpenPrHandoff(); + const handoffSeq = handoff?.seq ?? null; + useEffect(() => { + if (handoffSeq === null) return; + const url = takeOpenPrHandoff(handoffSeq); + if (url !== null) navigate.openUrl(url); + }, [handoffSeq, navigate]); const { efforts } = boardEfforts(b); const effort = efforts.find((e) => e.steward.key === effortKey); if (effort === undefined) { @@ -1489,10 +1552,16 @@ const SidebarThreadRow = memo(function SidebarThreadRow({ * choose`; an approve-pr or merge keeps its Open PR / Approve|Merge buttons. * The row itself opens the detail view (BB-85), so there is no Details button. */ -function SidebarInlineNeed({ item, onChanged }: { item: SidebarNeedItem; onChanged: () => void }) { +function SidebarInlineNeed({ item, projectId, effortKey, onChanged }: { item: SidebarNeedItem; projectId: string; effortKey: string; onChanged: () => void }) { const rpc = useRpc(); const navigate = useBbNavigate(); const [state, setState] = useState<"idle" | "running" | { error: string }>("idle"); + // The fallback timer that opens the PR externally when the page never takes + // the handoff (a hidden panel, a chunk that failed to load) (BB-102). + const openPrFallback = useRef(null); + useEffect(() => () => { + if (openPrFallback.current !== null) window.clearTimeout(openPrFallback.current); + }, []); const text = item.type === "need" ? item.what : item.question; const isApprovePr = item.type === "need" && item.kind === "approve-pr"; const isMerge = item.type === "need" && item.kind === "merge"; @@ -1516,6 +1585,21 @@ function SidebarInlineNeed({ item, onChanged }: { item: SidebarNeedItem; onChang const choose = (choice: string) => run(rpc.call("stewardsNeedAction", { needId: item.id, action: "choose", note: null, choice })); const approve = () => run(rpc.call("stewardsNeedAction", { needId: item.id, action: "approve", note: null, choice: null })); const merge = () => run(rpc.call("stewardsNeedAction", { needId: item.id, action: "merge", note: null, choice: null })); + // BB-102: hand the URL to the detail page, which is inside a pane and can + // open it in bb's in-app browser, then go there as the row does. If nothing + // takes the slot in time, fall back to the sidebar's own (external) openUrl. + const openPr = () => { + const url = item.url; + if (url === null) return; + if (openPrFallback.current !== null) window.clearTimeout(openPrFallback.current); + const seq = setOpenPrHandoff(url); + navigate.toPluginPanel(BOARD_PATH, { subPath: boardSubPath(projectId, effortKey) }); + openPrFallback.current = window.setTimeout(() => { + openPrFallback.current = null; + const pending = takeOpenPrHandoff(seq); + if (pending !== null) navigate.openUrl(pending); + }, OPEN_PR_HANDOFF_MS); + }; const button = "rounded-md border border-input px-2 py-0.5 text-[11px] hover:bg-state-hover disabled:opacity-50"; return (
@@ -1530,7 +1614,7 @@ function SidebarInlineNeed({ item, onChanged }: { item: SidebarNeedItem; onChang ) : isApprovePr || isMerge ? ( <> {item.url !== null ? ( - ) : null} @@ -1688,7 +1772,7 @@ const SidebarEffortRow = memo(function SidebarEffortRow({ {mine.length > 0 ? (
{mine.map((item) => ( - + ))}
) : null} diff --git a/plugins/stewards/sidebar-app.test.tsx b/plugins/stewards/sidebar-app.test.tsx index b5f723c..9ce94e0 100644 --- a/plugins/stewards/sidebar-app.test.tsx +++ b/plugins/stewards/sidebar-app.test.tsx @@ -8,7 +8,7 @@ import { loadPluginApp, renderSlot, type CapturedPluginApp, type RenderedSlot } import type { PluginSidebarThread } from "@get-bb/plugin-sdk/app"; import { PROJECT_CHANGED } from "./channels.js"; import { setBoardRoute } from "./board-route.js"; -import { rowRenderCounts } from "./app.js"; +import { resetOpenPrHandoff, rowRenderCounts } from "./app.js"; const thread = (over: Partial & { id: string }): PluginSidebarThread => ({ projectId: "p1", @@ -118,6 +118,35 @@ const needItem = (over: Record = {}) => ({ ...over, }); +const boardSteward = (over: Record = {}) => ({ + key: "ENG-1", + url: null, + title: "Fix login", + kind: "ticket", + phase: null, + stage: null, + threadId: "thr_steward", + summary: null, + nextAction: null, + actor: null, + waitingOn: null, + priority: null, + hold: null, + holdBehind: [], + needsUser: false, + stuck: false, + deployment: null, + pending: [], + requeueAt: null, + ending: null, + statusAt: null, + createdAt: 1_000, + wokeAt: null, + advancedAt: null, + workers: [], + ...over, +}); + const withUserItem = (over: Record = {}) => ({ type: "withUser", id: "esc_1", @@ -157,6 +186,7 @@ const renderSidebar = async (options: Parameters[2] = {}, pro afterEach(() => { app = null; + resetOpenPrHandoff(); setBoardRoute({ projectId: null, key: null }); delete (document as unknown as { startViewTransition?: unknown }).startViewTransition; }); @@ -1152,6 +1182,70 @@ describe("the board route store (BB-94)", () => { }); }); +// BB-102: the sidebar's Open PR button hands the PR URL to the effort detail +// page, which sits inside a pane and can open it in bb's in-app browser. +describe("the sidebar's Open PR handoff to the detail page (BB-102)", () => { + const prUrl = "https://github.com/acme/web/pull/1"; + const detailBoard = () => ({ + projects: [{ projectId: "p1", name: "Acme" }], + board: { + projectId: "p1", + name: "Acme", + paused: false, + projectStewardThreadId: null, + stages: [], + digest: null, + questions: [], + withUser: [], + needs: [], + lessons: [], + lessonCounts: { candidate: 0, trial: 0, promoted: 0, retired: 0 }, + stewards: [boardSteward()], + stuck: [], + ended: [], + }, + }); + const options = { + sidebarThreads: { projects, threads: [thread({ id: "thr_steward", displayTitle: "ENG-1: Fix login" })] }, + rpc: { + stewardsSidebar: () => sidebarData({ bindings: [binding({ needsUser: true })], needs: { count: 1, items: [needItem()] } }), + stewardsBoard: () => detailBoard(), + } as never, + }; + const renderList = (captured: CapturedPluginApp) => + renderSlot(captured.threadLists[0]!, { activeThreadId: null, activeProjectId: "p1", isCompactViewport: false, onNavigate: () => {}, searchQuery: "" }, options); + + it("records the navigation in the sidebar, then opens the PR once from the page", async () => { + const captured = await loadApp(); + const listSlot = renderList(captured); + fireEvent.click(await waitFor(() => listSlot.getByRole("button", { name: "Open PR" }))); + await waitFor(() => expect(listSlot.inspection.navigateCalls).toContainEqual({ method: "toPluginPanel", path: "board", options: { subPath: "p1/ENG-1" } })); + // The page mounts on the detail view the click navigated to and opens the + // PR from inside its pane. + const panelSlot = renderSlot(captured.navPanels[0]!, { subPath: "p1/ENG-1" }, options); + await waitFor(() => expect(panelSlot.inspection.navigateCalls).toContainEqual({ method: "openUrl", url: prUrl })); + expect(panelSlot.inspection.navigateCalls.filter((call) => call.method === "openUrl")).toHaveLength(1); + // The handoff is one-shot: a remount records no second open. + panelSlot.lifecycle.unmount(); + const remount = renderSlot(captured.navPanels[0]!, { subPath: "p1/ENG-1" }, options); + await waitFor(() => expect(remount.getByText("Fix login")).toBeTruthy()); + expect(remount.inspection.navigateCalls.filter((call) => call.method === "openUrl")).toHaveLength(0); + remount.lifecycle.unmount(); + listSlot.lifecycle.unmount(); + }); + + it("still opens the PR when Open PR is pressed while that detail view is showing", async () => { + const captured = await loadApp(); + const panelSlot = renderSlot(captured.navPanels[0]!, { subPath: "p1/ENG-1" }, options); + await waitFor(() => expect(panelSlot.getByText("Fix login")).toBeTruthy()); + const listSlot = renderList(captured); + fireEvent.click(await waitFor(() => listSlot.getByRole("button", { name: "Open PR" }))); + await waitFor(() => expect(panelSlot.inspection.navigateCalls).toContainEqual({ method: "openUrl", url: prUrl })); + listSlot.lifecycle.unmount(); + panelSlot.lifecycle.unmount(); + }); +}); + // BB-91: the sidebar's CSS-only row transitions. describe("the sidebar's row transitions (BB-91)", () => { // BB-95: the root view transition is gone, so no row carries a name for it. From a532ce7893890de7df0d96e2a6c7d32dd47e0d68 Mon Sep 17 00:00:00 2001 From: Dylan Trotter Date: Wed, 28 Oct 2026 20:12:57 +0000 Subject: [PATCH 2/2] stewards: update the inline Open PR design note for BB-102 --- plugins/stewards/DESIGN.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/plugins/stewards/DESIGN.md b/plugins/stewards/DESIGN.md index bb51b2b..657b3d9 100644 --- a/plugins/stewards/DESIGN.md +++ b/plugins/stewards/DESIGN.md @@ -1712,7 +1712,9 @@ question or need's text and its one-click actions, the same RPC calls the old block made. A fixed-choice question answers through `stewardsAnswerQuestion { choice }`, a `decide` need through `stewardsNeedAction { action: "choose", choice }`; an `approve-pr` row carries -**Open PR** (`navigate.openUrl` on the item's `url`) and **Approve** +**Open PR** (records the item's `url` in the plugin's one-shot handoff and +navigates to the effort's detail view, whose `navigate.openUrl` opens it from +inside the pane, BB-102) and **Approve** (`stewardsNeedAction { action: "approve" }`, no confirmation, the server's refusal shown in the row); a `merge` row swaps **Merge** for **Approve** (the lane gate re-checked server-side). There is no **Details** button: the row