stewards: open the sidebar's PR from the detail page's pane (BB-102)

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

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<typeof rpcContract>();
   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<number | null>(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 (
     <div className="flex flex-col gap-1">
@@ -1530,7 +1614,7 @@ function SidebarInlineNeed({ item, onChanged }: { item: SidebarNeedItem; onChang
         ) : isApprovePr || isMerge ? (
           <>
             {item.url !== null ? (
-              <button type="button" className={button} onClick={() => navigate.openUrl(item.url!)}>
+              <button type="button" className={button} onClick={openPr}>
                 Open PR
               </button>
             ) : null}
@@ -1688,7 +1772,7 @@ const SidebarEffortRow = memo(function SidebarEffortRow({
       {mine.length > 0 ? (
         <div className="flex flex-col gap-1 pb-1" style={{ paddingLeft: "40px" }}>
           {mine.map((item) => (
-            <SidebarInlineNeed key={`${item.type}:${item.id}`} item={item} onChanged={onChanged} />
+            <SidebarInlineNeed key={`${item.type}:${item.id}`} item={item} projectId={projectId} effortKey={key} onChanged={onChanged} />
           ))}
         </div>
       ) : null}

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<PluginSidebarThread> & { id: string }): PluginSidebarThread => ({
   projectId: "p1",
@@ -118,6 +118,35 @@ const needItem = (over: Record<string, unknown> = {}) => ({
   ...over,
 });
 
+const boardSteward = (over: Record<string, unknown> = {}) => ({
+  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<string, unknown> = {}) => ({
   type: "withUser",
   id: "esc_1",
@@ -157,6 +186,7 @@ const renderSidebar = async (options: Parameters<typeof renderSlot>[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.