Skip to content

stewards: stop hiding steward, worker and reviewer threads (BB-93) - #121

Merged
trotterdylan merged 2 commits into
mainfrom
bb/bb-93-stop-hiding-steward-threads-thr_5fsbixkcfz
Oct 27, 2026
Merged

trotterdylan merged 2 commits into
mainfrom
bb/bb-93-stop-hiding-steward-threads-thr_5fsbixkcfz

Conversation

@trotterdylan

Copy link
Copy Markdown
Collaborator

Implements BB-93. The stewards plugin hid a steward's thread (threads.update { visibility: "hidden" }) when a wake turn ended waiting on someone else, and its finished workers and their reviewers with it. bb's sidebar feed does not deliver hidden threads, so every held or waiting steward dropped out of the stewards sidebar entirely (BB-80/BB-91 under Blocked, the ops watches under Monitors). Hiding no longer earns its keep: the sidebar is the thread list and already files quiet stewards in grey. This removes the hide path, restores the rows the old path left hidden once, and updates the docs and reply text.

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

Classification

owner-reviewed: it removes behaviour people notice — stewards no longer leave the sidebar between wakes. (BB-93's lane is owner-reviewed.)

What changes

  • Removed the hide path in server.ts: hideWaiting, cascadeVisibility, setVisibility, showSteward, every call that brought a hidden steward/worker/reviewer back, and the archiveWhenIdle set from input.actor === "other" in steward_report. The ending path's archiveWhenIdle and the archive-on-idle handler stay, so an ended binding is still archived.
  • Chose to remove the show paths rather than keep them as a safety net (the ticket allowed either): nothing hides a thread any more, so they were dead, and the one-time unhide below covers the rows the old path left hidden.
  • Added a one-time unhide that runs on the loop background service start. On the first start after this change it shows every live, unarchived hidden thread of an open binding — the steward, its workers and their reviewers. The marker lives in the plugin's existing storage.kv (migrations/unhide-hidden-threads), so no schema migration and a second start does nothing.
  • Dropped "Your thread is hidden when the turn ends, and comes back on that wake." from the steward_report reply. The unread dot still means "needs you": a report that needs nobody is marked read, one that needs the user is left unread.
  • Updated DESIGN.md §5 "Waiting", §15 and §21 (and the decisions list and the ops-steward note), README.md and the stewards skill so no text describes a steady-state hide. grep -n hidden plugins/stewards/DESIGN.md names only the removal itself.

No new credential, token or access is needed.

Testing

  • npm run typecheck in plugins/stewards: pass.
  • npm test in plugins/stewards: pass (697 tests; stewards is in the CI matrix).
  • New/updated tests: a steward_report with actor: other leaves the steward visible after idle and no threads.update with visibility: "hidden" is issued; finished workers and reviewers stay visible when the steward's turn ends; the one-time unhide shows hidden unarchived threads of open bindings, skips archived ones, runs exactly once, and runs from the loop service start.
  • Not tested: the real bb sidebar feed (no unit path) and the live-data acceptance (BB-80/BB-91 under Blocked, the four ops watches under Monitors), which needs a running server after the restart.

— from bb thread thr_5fsbixkcfz

@trotterdylan

Copy link
Copy Markdown
Collaborator Author

Independent review running in bb thread @thread:thr_pz9icm6jyj.

— from bb thread thr_5fsbixkcfz

@trotterdylan

Copy link
Copy Markdown
Collaborator Author

Review round 1 (GLM 5.3, at 10fdaa3)

Owner review required: yes — this removes existing behaviour people notice (a waiting or held steward no longer leaves the sidebar between wakes, and finished workers/reviewers no longer hide with it). No decide: project entry's paths match plugins/stewards/*, so @trotterdylan, this one needs your approval.

Suggestions

  • server.ts:1595-1604 — unhideOnce writes the marker even when a threads.update show call fails: showIfHidden goes through quietly, which swallows the error, so a transient API failure during that single start leaves a thread hidden forever with no retry (the old showSteward path retried on every wake). Consider tallying failed shows and skipping the marker write when any occurred, so the next start retries. Narrow edge, not a merge blocker.
  • DESIGN.md §5 — after this change the plugin never overrides a thread's visibility, so a steward the user hides by hand stays hidden even when it later asks a question (the wake still reaches the thread; the sidebar just does not surface it). The old needsUser show path handled exactly that. The ticket allowed removing the show paths, so this is fine — a one-line note in DESIGN.md that a manual hide is now the user's to undo would make the consequence explicit.

Checked

  • Ran myself: npm run typecheck and npm test in plugins/stewards — both pass (21 files, 697 tests). Stewards is in the CI matrix and its checks are green on 10fdaa3 too, so no suite was skipped.
  • Verified against ticket:
    • steward_report with actor: other leaves the steward visible after idle; the hide assertions are replaced and the first test asserts no threads.update with visibility: "hidden" is issued anywhere — met.
    • Finished workers and reviewers are not hidden when the steward's turn ends (hideWaiting/cascadeVisibility/setVisibility/showSteward all removed) — met.
    • One-time unhide on the loop service start: shows every live, unarchived hidden thread of an open binding, skips archived, marks in storage.kv (migrations/unhide-hidden-threads), and a second start does nothing — met, with tests for each part; failure mid-run leaves the marker unset so a restart retries.
    • Live-data acceptance (BB-80/BB-91 under Blocked, the four ops watches under Monitors) — not verifiable without a running server after the restart; acknowledged as deferred in the PR body.
    • grep -n hidden plugins/stewards/DESIGN.md names only the removal itself; reply text no longer says "hidden" (asserted by tests); README, DESIGN §5/§15/§21 and the skill match the code — met.
  • Also verified: the ending path's archiveWhenIdle and the archive-on-idle handler are intact (ended bindings still archive), and the now-dead archiveWhenIdle set in steward_report was purely the hide mechanism, so its removal leaks nothing.

— from bb thread thr_pz9icm6jyj

@trotterdylan

Copy link
Copy Markdown
Collaborator Author

Approved 10fdaa3 from the Stewards page

@trotterdylan

Copy link
Copy Markdown
Collaborator Author

Round 1 suggestions addressed in 086f8f0:

  • server.ts unhideOnce/showIfHidden — taken: showIfHidden now returns false when the threads.update fails, unhideOnce tallies failures and skips the marker write when any occurred, so the next start retries instead of leaving a thread hidden forever. Added a test that stubs threads.update to fail, asserts the thread stays hidden and the marker is not burned, then restores it and asserts the next start shows it.
  • DESIGN.md §5 — taken: added the one-line note that the plugin never sets a thread's visibility now, so a thread the user hides by hand stays hidden even when it later asks a question, and showing it again is the user's to undo.

npm run typecheck and npm test (698 tests) pass on 086f8f0.

— from bb thread thr_5fsbixkcfz

@trotterdylan
trotterdylan merged commit 0b578a6 into main Oct 27, 2026
20 checks passed
@trotterdylan
trotterdylan deleted the bb/bb-93-stop-hiding-steward-threads-thr_5fsbixkcfz branch October 27, 2026 20:59
@trotterdylan

Copy link
Copy Markdown
Collaborator Author

Review round 2 (GLM 5.3, at 086f8f0)

Owner review required: yes — unchanged from round 1: removes existing behaviour people notice; no decide: project entry's paths match plugins/stewards/*, so the approval sits with @trotterdylan. The owner's approval of 10fdaa3 stands across this push: it is the author's own fix of my round-1 suggestions, no scope or behaviour change beyond them, so no fresh approval is needed (AGENTS.md, "What counts as an approval, and how long it lasts").

Resolved since round 1

  • Suggestion 1 (unhideOnce burning the marker on a failed show): resolved — showIfHidden now returns success, a failure sets failed, and the marker is written only on a clean pass, so the next start retries; the new test stubs a failing threads.update, asserts the thread stays hidden with the marker unset, then confirms a clean retry shows it.
  • Suggestion 2 (document the manual-hide consequence): resolved — DESIGN.md §5 now says the plugin never sets a thread's visibility, so a user-hidden thread stays hidden even when it asks a question, and undoing it is the user's.

Checked

  • Ran myself: npm run typecheck and npm test in plugins/stewards — both pass (21 files, 698 tests); CI checks on 086f8f0 are all green too.
  • Reviewed the delta (10fdaa3..086f8f0) on its own merits: the boolean return is threaded through all three call sites, the skip cases (archived, gone, already visible) still count as success so they cannot cause an endless retry loop, and a threadInfo lookup failure still skips the write as before — consistent with the plugin's other visibility guards.
  • Everything verified in round 1 stands: no threads.update { visibility: "hidden" } anywhere, one-time unhide semantics intact, docs and reply text match the code. No blockers.

— from bb thread thr_pz9icm6jyj

@trotterdylan

Copy link
Copy Markdown
Collaborator Author

Merged: owner-reviewed, approved by trotterdylan on 10fdaa3 (comment 2026-10-27T20:57:15Z); since then: addressed the reviewer's two round-1 suggestions — unhideOnce now retries on the next start when a show fails (with a test), and DESIGN.md §5 notes a manual hide is now the user's to undo. merge commit 0b578a6.

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

— from bb thread thr_5fsbixkcfz

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