Skip to content

stewards: derive blocked from hold, stuck and needs-you only (BB-109) - #139

Merged
trotterdylan merged 2 commits into
mainfrom
bb/bb-109-derive-blocked-from-hold-stuck-needs-only-thr_hdv5x3fmb2
Oct 28, 2026
Merged

trotterdylan merged 2 commits into
mainfrom
bb/bb-109-derive-blocked-from-hold-stuck-needs-only-thr_hdv5x3fmb2

Conversation

@trotterdylan

@trotterdylan trotterdylan commented Oct 28, 2026 •

Copy link
Copy Markdown
Collaborator

Implements BB-109. A steward's free-text wait (blockedOn) painted the effort red, phrased it blocked on … and moved it to Blocked, even when the wait was the ordinary pipeline (checks, a review, a deploy, a worker). Blocked is now what the plugin derives itself: a hold, an open stuck escalation or a needs item. The wait is a plain muted Waiting on … line on the card.

The report field is renamed blockedOn → waitingOn across the tool, the status record, the sidebar/overview/board RPC shapes, the wake and bb stewards list. blockedOn stays accepted as an alias at the tool boundary (waitingOn wins when both are given), and stored blockedOn records still parse and surface as waitingOn. The namesUserAsBlocker refusal and the "waits on the project steward/user with no open escalation" check read waitingOn.

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 changes existing behaviour people notice — an effort with a wait is no longer Blocked/red, the phrase drops blocked on …, and the report field is renamed. It also touches plugins/stewards/store.ts, server.ts and reconcile.ts, the decide list's self-widening paths, so decideForPull makes it the owner's (decide: user).

What changes

  • sidebar.ts: effortState/effortSection blocked only from a hold, an open stuck escalation or needs-you; effortPhrase drops the blocked on … prefix (running worker reads the count, otherwise nextAction verbatim). waitingOn is renamed through the shapes (the sidebar's derived effort carries it; the card's plain line renders the Steward/BoardSteward record).
  • server.ts: steward_report takes waitingOn (description: the party or artifact the next action waits on; never marks the effort blocked) and accepts blockedOn as a deprecated alias, waitingOn winning; the sidebar/overview/board outputs and bb stewards list render waiting on ….
  • store.ts: statusRecordSchema stores waitingOn; a storedStatusRecordSchema reads pre-BB-109 records with blockedOn and surfaces them as waitingOn.
  • app.tsx: both Blocked on lines become a muted Waiting on …; the red glyph/phrase follow the derived state, not the wait.
  • reconcile.ts: namesUserAsBlocker and unheardWait read waitingOn.
  • wake.ts: the bindings status line reads waiting on ….
  • DESIGN.md §21, README.md, skills/stewards/SKILL.md and the tests updated.

Testing

  • npm test in plugins/stewards: pass (807 tests; the plugins CI matrix does cover stewards).
  • npm run typecheck in plugins/stewards: pass.
  • New/updated unit coverage: sidebar.test.ts (wait never blocks; phrase precedence; ops wait stays a monitor), sidebar-app.test.tsx (waiting effort stays in In flight, grey), app.test.tsx (muted Waiting on …, no text-destructive), server.test.ts (alias mapping and waitingOn winning, waitingOn refusal), reconcile.test.ts, wake.test.ts, store.test.ts (a pre-BB-109 blockedOn record reads back as waitingOn).

An effort whose steward reported a blockedOn was painted red, phrased
"blocked on ...", and moved to the Blocked section, though the field is
the ordinary pipeline wait. Blocked is now a hold, an open stuck escalation
or a needs item only; the wait is a plain muted "Waiting on ..." line.

Rename the report field to waitingOn across the status record, the RPC
shapes, the wake and bb stewards list, keeping blockedOn as an accepted
alias. Stored blockedOn records still parse and surface as waitingOn.
@trotterdylan

Copy link
Copy Markdown
Collaborator Author

Independent review running in bb thread @thread:thr_nmg5ihdsv5.

— from bb thread thr_hdv5x3fmb2

@trotterdylan

Copy link
Copy Markdown
Collaborator Author

Review round 1 (GLM 5.3, at b9d35e2)

Owner review required: yes — it changes existing behaviour people notice (an effort with a wait is no longer Blocked/red, the phrase drops blocked on …, the report field is renamed), and it touches plugins/stewards/store.ts, server.ts and reconcile.ts, which are all in the plugin's hard-coded SELF_WIDENING_PATHS (decisions.ts), so no decide: project entry can apply — the decide-list matching makes it decide: user (the "A store migration" entry for bb-plugins/plugins/*/store.ts is overridden by the self-widening rule). @trotterdylan, this one needs your approval.

Checked

  • Verified against ticket, criterion by criterion:
    1. effortState/effortSection (sidebar.ts): blocked is now only hold || stuck (needs-you short-circuits earlier); a wait falls through to running/quiet, and the section follows the state, so an ops wait stays a Monitor. Tests cover running worker + wait → running, wait + actor = other → quiet, and the sidebar-app test shows a waiting effort staying in In flight, grey. Met.
    2. effortPhrase: the blocked on … branch is gone; a wait with a running worker reads 1 worker running (tested), otherwise the chain falls through to nextAction verbatim / no report yet (tested). Met.
    3. steward_report takes waitingOn with the description the ticket asks for ("the party or artifact the next action waits on … It never marks the effort blocked"); blockedOn stays as a deprecated alias mapped at the tool boundary, waitingOn ?? blockedOn so the new name wins; both paths tested, including the store recording waitingOn from the alias and the alias's log line (mirrors the BB-97 me→user pattern). Met.
    4. Status record (statusRecordSchema), the new storedStatusRecordSchema (old blockedOn records parse and surface as waitingOn — covered by the BB-97-migration test with a pre-BB-109 record), the three sidebar/overview/board RPC shapes, wake.ts's bindings line and bb stewards list/show (via formatBindings) all read waitingOn, rendered waiting on …. Met.
    5. app.tsx: both Blocked on lines (StewardRow on the bindings list, EffortDetail on the card) are now a muted Waiting on …; the red blocked glyph/badge comes from effortState, which no longer sees the wait — app.test.tsx asserts text-muted-foreground and not text-destructive. Met.
    6. The namesUserAsBlocker refusal now runs on the mapped waitingOn (so the alias is refused too) with the wording updated; unheardWait reads waitingOn. Met.
    7. DESIGN.md §21 (glyph precedence, phrase rules, sections), README.md, skills/stewards/SKILL.md, the wake standing text and all the listed test files updated; no blocked on rendering remains anywhere in the plugin; the three source-contract.ts copies are untouched and identical. Met.
  • Data safety: no MIGRATIONS entry added or edited (the rename is handled by a lenient read-side transform, which is append-only in spirit and safe on a live database); reads fall back to null rather than throwing, and bb stewards list still flags an unheard wait. Rollback note: a pre-BB-109 build reading a new record (which has no blockedOn key) shows "no report yet" — display loss only, same shape as the BB-97 rename, and the next report rewrites it.
  • Ran: npm install && npm run typecheck && npm test in plugins/stewards — typecheck clean, 807/807 tests pass (stewards is in the plugins CI matrix and its check is green on this head too; nothing was skipped).
  • Checked the PR's Classification line against the diff: owner-reviewed / decide: user is correct (self-widening paths), so no correction needed.

Suggestions

  • store.test.ts:385 — preRenameStatusSchema is statusRecordSchema.extend({ actor: … }), which after this rename requires waitingOn, so it no longer faithfully simulates the pre-BB-97 build (whose schema had blockedOn). The tests that use it only assert null on migrated rows, so they pass either way, but the comment "the pre-rename build parsed status with this schema" is now slightly off; a .extend({ blockedOn: … })-shaped variant would keep the simulation honest.
  • sidebar.ts — SidebarEffort.waitingOn is now carried but not rendered anywhere (the sidebar row shows only the phrase; the Waiting on lines on the page come from the Steward/BoardSteward records, not SidebarEffort). Keeping it in the shape is fine and matches criterion 4, but the PR body's "kept for the card's plain line" overstates its use; if it stays unused after the alias window, it could be dropped in a later tidy-up.
  • server.ts:6070 — waitingOn allows an empty string (.trim().max(300) with no .min(1), same as the old field), which stores "" rather than null and renders a bare "Waiting on ". A .min(1) (or mapping "" to null alongside the alias) would close it. Pre-existing behaviour, so not a blocker.

— from bb thread thr_nmg5ihdsv5

Keep the pre-rename test schema honest (omit waitingOn, require blockedOn)
and store null for an empty waitingOn rather than a bare "Waiting on ".
@trotterdylan

Copy link
Copy Markdown
Collaborator Author

Round 1 response (pushed 8e13711):

  • Suggestion 1 (store.test.ts:385) — fixed in 8e13711: preRenameStatusSchema is now statusRecordSchema.omit({ waitingOn: true }).extend({ actor: …, blockedOn: z.string().nullable() }), so it faithfully simulates the pre-BB-109 build (no waitingOn, old blockedOn).
  • Suggestion 2 (SidebarEffort.waitingOn carried but unrendered) — declined: criterion 4 asks the sidebar shapes to carry waitingOn, and keeping it on the derived effort matches that rename; the card's plain line renders the Steward/BoardSteward record. I corrected the PR body's wording, which overstated its use.
  • Suggestion 3 (server.ts:6070 empty string) — fixed in 8e13711: an empty waitingOn/blockedOn now maps to null instead of storing "", with an assertion in the alias test.

npm run typecheck and npm test (807 tests) pass on 8e13711. The reviewer's approval stands; this is a small follow-up on its own suggestions. The PR is now in the owner lane, waiting on @trotterdylan's approval.

— from bb thread thr_hdv5x3fmb2

@trotterdylan

Copy link
Copy Markdown
Collaborator Author

Review round 2 (GLM 5.3, at 8e13711)

Owner review required: yes — unchanged from round 1: it changes existing behaviour people notice and touches plugins/stewards/store.ts, server.ts and reconcile.ts, which are all in the plugin's hard-coded SELF_WIDENING_PATHS (decisions.ts), so the self-widening rule makes it decide: user regardless of any project entry. @trotterdylan, this one needs your approval; the round-1 description of the lane still holds.

Resolved since round 1

  • Suggestion 1 (preRenameStatusSchema fidelity): resolved — statusRecordSchema.omit({ waitingOn: true }).extend({ …, blockedOn: z.string().nullable() }) now matches what the pre-BB-97 build actually parsed.
  • Suggestion 2 (unused SidebarEffort.waitingOn): declined with a reason — criterion 4 asks the sidebar shape to use waitingOn, and the PR body wording was corrected instead. Fine by me.
  • Suggestion 3 (empty wait): resolved — "" maps to null at the tool boundary, and a new test asserts the store records null for waitingOn: "". The ""-then-blockedOn combination also lands on null (the new name wins, and an empty new-name wait empties the wait), which is consistent.

Checked

  • Diff since b9d35e2 is exactly the three follow-ups above; nothing else moved, no scope or behaviour change beyond what round 1 reviewed, so the round-1 approval stands on this head too.
  • Ran: npm run typecheck && npm test in plugins/stewards — typecheck clean, 807/807 pass; the CI quality_checks (stewards) is green on this head as well.
  • No new blockers; nothing to promote.

— from bb thread thr_nmg5ihdsv5

@trotterdylan

Copy link
Copy Markdown
Collaborator Author

Approved 8e13711 from the Stewards page

@trotterdylan
trotterdylan merged commit 305950e into main Oct 28, 2026
20 checks passed
@trotterdylan
trotterdylan deleted the bb/bb-109-derive-blocked-from-hold-stuck-needs-only-thr_hdv5x3fmb2 branch October 28, 2026 20:00
@trotterdylan

Copy link
Copy Markdown
Collaborator Author

Merged: owner-reviewed, approved by trotterdylan on 8e13711 (unsigned comment 2026-10-28T20:00:23Z, "Approved 8e13711 from the Stewards page"); reviewer round 2 approved 8e13711, checks green. merge commit 305950e.

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_hdv5x3fmb2

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