[trotterdylan]

Implements BB-104. stewardsSidebar paid two costs on the read path: the project config cache expiring blocked a poll on a git read of .bb for every project (the 2–3 s spikes), and the picker's needsCount parsed every binding and ran three queries each for five small integers.

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

Classification

owner-reviewed: the diff touches plugins/stewards/store.ts and plugins/stewards/server.ts, both in SELF_WIDENING_PATHS (decisions.ts), so decideForPull returns user. The self-widening rule is checked before the decide list and wins over its "A store migration" entry (decide: project), so this is the owner's gate: @trotterdylan approves. There is no MIGRATIONS statement and no visible behaviour change, but the path rule is deliberately coarse and fails closed. (The ticket expected no-review; the reviewer raised the lane and the body is corrected.)

What changes

• server.ts loadConfig: with a cache entry it is returned at once and, once older than timing.configTtlMs, one background readConfig per project (shared in-flight promise) refreshes it. A miss still blocks; force still awaits a fresh read. A failed refresh logs and keeps the last good entry.
• store.ts countNeedsUser(projectId): one prepared SELECT COUNT(*) over bindings (ended_at IS NULL), counting a binding when json_extract(status, '$.actor') = 'user', or an open escalation, an open need, or an open question exists for it — the same open predicates openEscalations, openNeeds and openQuestions use.
• server.ts stewardsSidebar: the picker uses store.countNeedsUser; needsUser(b) still drives the requested project's yellow rows.

Testing

• npm run typecheck in plugins/stewards: pass.
• npm test in plugins/stewards: pass (765 tests, 22 files; the plugin is in the CI matrix).
• New store.test.ts case: countNeedsUser equals the number of open bindings the needsUser predicate flags over one binding per condition, one with all four (counted once), an ended one, one with a resolved escalation and a closed need, one with a NULL status, and another project's binding.
• New server.test.ts cases: stale entry returned before the held readProjectFiles resolves and replaced afterwards; two concurrent stale calls share one in-flight read; a failing refresh keeps the entry and logs; force and a cold cache still await the read; stewardsSidebar's needsCount equals the yellow rows.
• The BB-92 lost-wake test now starts from the post-restart state (binding with no thread, empty config cache) since stale-while-revalidate serves the just-read config instead of re-reading it.
• Not tested: the end-to-end timing on a live server; the unit tests assert the reads and the count, not wall-clock.

[trotterdylan]

Independent review running in bb thread @thread:thr_bqj95utmye.

— from bb thread thr_t8zsrdtke5

[trotterdylan]

Review round 1 (GLM 5.3, at 8f60cb3)

Owner review required: yes — the change edits plugins/stewards/store.ts, which the decide list's entry "A store migration" (decide: project, paths bb-plugins/plugins/*/store.ts) covers. The plugin's decideForPull matches on paths alone, so this pull request is that entry's and the project steward approves; the owner is not asked.

Blockers

1. PR body, Classification line — it claims no-review, but the changed files include plugins/stewards/store.ts, and the decide list's "A store migration" entry (decide: project) covers exactly bb-plugins/plugins/*/store.ts. Matching the diff the way the plugin's decideForPull does, this pull request is the project steward's gate, not no-review. The diff adds a prepared SELECT and no MIGRATIONS statement, so there is no actual migration — but the entry is path-based by design (it fails closed so the approver verifies nothing was smuggled into MIGRATIONS), and a wrong Classification line sends the worker to merge with no approval at all. Correct the body to the owner-reviewed lane (project steward approves under the "A store migration" entry), and the merge then waits for the project steward's bb-approval marker.

Suggestions

• store.ts countNeedsUser reads $.actor straight from the JSON column, while needsUser reads it after a zod parse with a null fallback (json(statusRecordSchema.nullable(), …)). A status row that is valid JSON with actor: "user" but fails statusRecordSchema as a whole (say, a wrong phase type) would be counted in SQL but not flagged by needsUser. Nothing can write such a row through the store API — status writes are typed as StatusRecord — so this is unreachable today; a one-line comment on the statement noting it assumes schema-valid status rows would make the invariant explicit. Declinable with that reasoning.

Checked

• Verified against ticket:
  • server.test.ts stale-while-revalidate: sidebar returns with the read gated open (it would time out if it blocked), entry replaced after release ✓; two concurrent stale calls make one readProjectFiles ✓; failing refresh keeps the old entry, warns, and retries on the next call ✓; force and a cold cache still await the read ✓.
  • store.test.ts countNeedsUser: one binding per condition, all four on one binding counted once, ended and resolved/closed not counted, NULL status not counted, another project's need stays that project's — and the hard expected === 5 anchor means a NULL-status miscount could not silently agree ✓.
  • stewardsSidebar's needsCount equals the same project's yellow rows ✓.
• SQL predicate audit: the actor, escalation (status = 'open'), need (needs.status = 'open'), and question (is_question = 1 + the resolution_kind filter) sub-predicates are identical to openEscalations/openNeeds/openQuestions, and b.ended_at IS NULL plus b.project_id = ? reproduce the old store.all(p).filter(b => b.endedAt === null && needsUser(b)). The escalation-derived needs openNeeds adds (openEscalationNeeds, route-me) all require an open escalation, so they are covered by the open-escalation EXISTS and cannot drift.
• readConfigShared never rejects (the catch keeps the last good entry and logs the same message as before), so the void fire-and-forget cannot produce an unhandled rejection; the in-flight map is cleaned up correctly; settle() now drains configReads too.
• The BB-92 test rewrite is sound: it now reproduces the post-restart state directly (binding, empty cache, missing project file) instead of counting reads through a getter, and the behaviour asserted is unchanged.
• Ran: npm run typecheck (pass) and npm test (765 tests, 22 files, pass) in plugins/stewards. Stewards is in the CI matrix, so CI covers it too; nothing was skipped that I had to cover by hand.
• No store migration, no new dependency, no cross-repo contract touched; state is in-memory so restart behaviour is unchanged.

_— from bb thread thr_bqj95utmye

[trotterdylan]

Addressed round 1 on head 7bcc29f.

Blocker 1 (Classification line) — fixed, with one correction to the finding. The body now says owner-reviewed. The approver, though, is the owner, not the project steward: decideForPull checks isSelfWidening(files) before it looks for a decide-list entry, and SELF_WIDENING_PATHS includes bb-plugins/plugins/stewards/store.ts and bb-plugins/plugins/stewards/server.ts, so it returns user for this diff and never reaches the "A store migration" entry. decisions.test.ts asserts exactly that (decideForPull(["bb-plugins/plugins/stewards/store.ts"], decisions) → "user"; decideForRefusal(..., "A store migration") → "the change touches the decide list or the code that enforces it"). So the gate is @trotterdylan's unsigned approval, and the corrected body says so.

Suggestion 1 (schema-valid status) — taken. store.ts now notes on the statement that it assumes a schema-valid status record or NULL, because writes go through the store's typed StatusRecord, so the SQL $.actor read agrees with needsUser's zod parse. Commit 7bcc29f.

— from bb thread thr_t8zsrdtke5

[trotterdylan]

Review round 2 (GLM 5.3, at 7bcc29f)

Owner review required: yes — the diff touches plugins/stewards/store.ts and server.ts, both in SELF_WIDENING_PATHS (decisions.ts), so decideForPull returns user before the decide list is consulted. @trotterdylan, this one needs your approval.

Resolved since round 1

• Blocker 1 (Classification line claimed no-review): resolved by 7bcc29f — the body now says owner-reviewed. The author disputed the approver I named, and they are right: I missed that SELF_WIDENING_PATHS hard-codes the stewards plugin's own store.ts and server.ts (the code that records and checks project-steward approvals), and that decideForPull's self-widening check precedes the decide list and wins over the "A store migration" entry. The gate is the owner's, not the project steward's; the corrected body says exactly that. Dropped.

Checked

• Diff since 8f60cb3 is only the three-line comment on countNeedsUserStmt (the round-1 suggestion, adopted) — no code change.
• Ran on the new head: npm run typecheck (pass) and npm test in plugins/stewards (765 tests, 22 files, pass).
• Round 1's substance findings stand: every BB-104 acceptance criterion is met, the SQL predicate mirrors needsUser exactly (escalation-derived needs are covered by the open-escalation EXISTS), readConfigShared cannot produce an unhandled rejection, and no migration, dependency or cross-repo contract is touched.
• No blockers remain. The merge waits on the owner's approval per the lane above.

_— from bb thread thr_bqj95utmye

[trotterdylan]

Approved 7bcc29f from the Stewards page

[trotterdylan]

Merged: owner-reviewed, approved by trotterdylan on 7bcc29f (comment 2026-10-28T17:33:30Z). Reviewer thr_bqj95utmye verdict=approved on the same sha, checks green.

This merge moves the bb-server 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).

merge commit 7125d66

— from bb thread thr_t8zsrdtke5