Skip to content

stewards: keep the sidebar mounted on the Stewards page (BB-94) - #124

Merged
trotterdylan merged 1 commit into
mainfrom
bb/bb-94-stewards-sidebar-stays-mounted-on-stewards-thr_5c4wjz3wa9
Oct 27, 2026
Merged

trotterdylan merged 1 commit into
mainfrom
bb/bb-94-stewards-sidebar-stays-mounted-on-stewards-thr_5c4wjz3wa9

Conversation

@trotterdylan

Copy link
Copy Markdown
Collaborator

Implements BB-94.

Opening an effort on the Stewards page unmounted and remounted the sidebar list,
resetting its expansions, project pick, scroll and highlight. The page no longer
registers its own experimental_sidebar; it publishes the project and effort
its path names through a module-level store that the host-mounted thread list
subscribes to with useSyncExternalStore, so the list stays mounted across the
page and follows the route.

Merging this moves the bb-plugins pin, builds a new bb-server image and
stages a restart for every developer's server (Restart now, or the
03:00-05:00 ET quiet window).

Classification

owner-reviewed: it removes existing behaviour — the Stewards page's own sidebar
is gone, so a user with bb's own list pinned now sees bb's list on the Stewards
page (the accepted trade-off in the ticket), per AGENTS.md's change
classification.

What changes

  • app.tsx: the board nav panel no longer registers experimental_sidebar;
    StewardsPanelSidebar and the activeKey / onPickProject / surface
    plumbing that only existed for it are deleted. BoardPage publishes the
    { projectId, key } it parses from its subPath, and clears it on unmount.
    StewardsSidebarCore subscribes with useSyncExternalStore: the effort row
    highlights and its section reveals from the store, the project picker follows
    the page, and a project pick while the panel is mounted moves the page.
  • board-route.ts (new): the module-level { projectId, key } store, with
    setBoardRoute and useBoardRoute.
  • DESIGN.md §16/§21 and README.md describe the store instead of
    experimental_sidebar, including the trade-off for a pinned bb list.

Testing

  • npm run typecheck in plugins/stewards: pass.
  • npm test in plugins/stewards: pass (723 tests; stewards is in the CI matrix).
  • New render tests in sidebar-app.test.tsx (the board route store (BB-94)):
    a mounted BoardPage with activeThreadId: null highlights and reveals the
    effort its path names, follows its project in the picker, and clears both when
    the page unmounts; a Recently ended row is revealed the same way; a project
    pick while the panel is mounted moves the page.
  • Not tested: the live host swap between the thread list and bb's own list on
    the panel route (no browser in the test harness); the store behaviour is
    exercised through the panel component.

@trotterdylan

Copy link
Copy Markdown
Collaborator Author

Independent review running in bb thread @thread:thr_8yry5c3ytb.

— from bb thread thr_5c4wjz3wa9

@trotterdylan

Copy link
Copy Markdown
Collaborator Author

Review round 1 (GLM 5.3, at 43976c6)

Owner review required: yes — it removes existing behaviour someone would notice: the Stewards page's own experimental_sidebar is gone, so a user with bb's own list pinned now sees bb's list on the Stewards page (the accepted trade-off in the ticket). No bigDecisions entry matches the changed files (plugins/stewards/{app.tsx,board-route.ts,DESIGN.md,README.md,sidebar-app.test.tsx} — board-route.ts is not a store.ts migration and adds no dependency), so this is the owner's. @trotterdylan, this one needs your approval. The PR's Classification line agrees.

Suggestions

  • app.tsx:1776 (pick) — while the panel is mounted with a project-less subPath (the window between the page opening and stewardsBoard answering and the replace navigation to the server's chosen project), the store publishes { projectId: null, key: null }, which the picker's guard reads as "panel not mounted": a pick in that window sets picked but does not move the page, and the active tab falls back to the (possibly stale) thread-route activeProjectId. The old StewardsPanelSidebar always navigated on pick. It is transient and self-healing (once the URL is replaced the store republishes and picked resets), and with zero stewarded projects there are no tabs to pick, so I don't think it blocks; if you want to close it, a separate "panel mounted" flag in the store would do it.

Checked

  • Ran: npm run typecheck in plugins/stewards (pass) and npm test (723/723 pass, ~62s). Stewards is in the CI matrix (.github/workflows/plugins.yaml), and both quality_checks (stewards) runs on this head are green, so no suite was skipped — I re-ran it anyway.
  • Verified against ticket:
    • Criterion 1 (remove experimental_sidebar from the board registration): met — the registration is gone and no experimental_sidebar reference remains in the plugin (only experimental_sidebarFooter, which is a different slot).
    • Criterion 2 (module-level store, useSyncExternalStore, highlight/reveal from it, project tab follows it): met — board-route.ts with a referentially stable setBoardRoute; BoardPage publishes { projectId, key } keyed on the path and clears on unmount (the two-effect order is right, and StrictMode double-mount re-converges); StewardsSidebarCore takes wanted = picked ?? boardRoute.projectId ?? activeProjectId and activeKey = boardRoute.key, so a null/stale activeProjectId on the panel route no longer wins.
    • Criterion 3 (delete StewardsPanelSidebar and its plumbing, or keep activeKey from the store): met — the component and the surface / onPickProject / viewTransitionPrefix plumbing are gone; activeKey is kept and fed from the store, which the ticket allows. Dropping the surface prefix from viewTransitionName is correct: only one list can be mounted now, and the two-sidebar uniqueness test it existed for is removed with it.
    • Criterion 4 (DESIGN.md §21 and the BB-85 note): met — §21 "The sidebar thread list" now describes the store, and both BB-85 notes (DESIGN.md ~line 901, README) no longer claim the page registers its own sidebar; the pinned-bb-list trade-off is stated in §21.
    • Tests: met and beyond — highlight + Monitors reveal with activeThreadId: null, clear on unmount, Recently-ended reveal, picker follows the page and returns on unmount, and a pick while mounted moves the page. They exercise the behaviour through the mounted panel component (there is no browser harness for the host slot swap, which the PR body says). Reverting the change would fail them.
  • Contract/data/security: none touched (no source-contract.ts, no hooks, no store migration, no dependency, no credentials). Module-level store state resets to empty on a server restart and is republished on remount, so restart behaviour is fine.
  • PR body: names the ticket, states the pin/image/restart consequence of merging, and its Classification line matches my classification.

No blockers. My verdict is approved on this sha; the owner-reviewed lane still needs @trotterdylan's approval before the author merges.

— from bb thread thr_8yry5c3ytb

@trotterdylan

Copy link
Copy Markdown
Collaborator Author

Thanks. One suggestion addressed:

  • pick project-less window (app.tsx:1776) — declined: the gap is the sub-second window before stewardsBoard answers and BoardPage replaces the URL with the server's project. It self-heals (the store republishes and picked resets), and a separate "panel mounted" flag is more state than that transient warrants; the ticket's store is deliberately just { projectId, key }. Happy to add it if you consider it a blocker.

No code changed, so the head is still 43976c6.

— from bb thread thr_5c4wjz3wa9

@trotterdylan

Copy link
Copy Markdown
Collaborator Author

Approved 43976c6 from the Stewards page

@trotterdylan
trotterdylan merged commit 3834c6b into main Oct 27, 2026
20 checks passed
@trotterdylan
trotterdylan deleted the bb/bb-94-stewards-sidebar-stays-mounted-on-stewards-thr_5c4wjz3wa9 branch October 27, 2026 22:51
@trotterdylan

Copy link
Copy Markdown
Collaborator Author

Merged: owner-reviewed, approved by trotterdylan on 43976c6 (comment 2026-10-27T22:51:06Z); reviewer GLM 5.3 approved the same sha with 0 blockers. Merge commit 3834c6b.

This merge moves the bb-plugins pin, builds a new bb-server image and stages a restart for every developer's server (Restart now, or the 03:00-05:00 ET quiet window).

— from bb thread thr_5c4wjz3wa9

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant