Repository navigation
stewards: needs --all shows a done row's resolution (BB-283) - #311
Conversation
|
Independent review running in bb thread @thread:thr_56djnwd3en. — from bb thread thr_jnbk7xzhny |
Review round 1 (glm-5p3, at 3fd262d)Owner review required: yes — the diff touches The done-row line is exactly the open-row head plus Tested
Looks good. — from bb thread thr_56djnwd3en |
|
Approved 3fd262d from the Stewards page |
3fd262d to
6686c57
Compare
|
Merged: owner-reviewed, approved by trotterdylan on 3fd262d (comment 2026-11-10T03:30:45Z); since then: rebased onto main after the stack base #309 merged (content unchanged, only the base moved). merge commit ed689ed Reviewer approved 3fd262d (glm-5p3, round 1, 0 blockers); checks green on 6686c57. Note: merging this moves the _— from bb thread |
…nto modules (BB-280) (#313) Implements [BB-280](https://linear.app/bit-complete/issue/BB-280/stewards-the-decide-list-stops-routing-the-plugins-own-evolution-to). PR 1 of two: a behaviour-identical refactor of `plugins/stewards` that pulls the authority seams out of `config.ts` and `server.ts`, so PR 2 can narrow the plumbing entry in `.bb/stewards/project.json` to the new modules alone. > **A merge to `bb-plugins` `main` moves the pin, builds a new `bb-server` image and stages a restart for every developer's server** (`.bb/AGENTS.md`, "Shipping a plugin change"). Anyone who never presses Restart now is updated in the quiet window, 03:00–05:00 US Eastern. Sibling PR: this is stacked on BB-279's [#312](#312) (base branch `bb/bb-279-pr-1-gate-merge-base-rule-and-owner-appro-thr_4kywa236ny`). It lands with BB-279; it must not be retargeted to `main` before that merges. ## Classification owner-reviewed: the decide-list entry "The stewards plugin's roles parsing, ready gate and approval and escalation plumbing outside the hard-coded self-widening list" (`decide: user`) matches `plugins/stewards/config.ts` and `plugins/stewards/server.ts`, and the extension also touches `plugins/stewards/approvals.ts`, one of the `SELF_WIDENING_PATHS` files, which is always the user's. This refactor moves that plumbing into `roles.ts`, `ready-gate.ts`, `escalations.ts` and `approvals.ts`; the moved code stays owner-reviewed under PR 2's narrowed entry. ## What changes No behaviour changes. The moved code is byte-for-byte the same logic, only relocated behind an explicit context object. - **`roles.ts` (new)** — all of the roles.json code from `config.ts`: the `roles.json` schemas, the V1/V2 normalisation, `PROJECT_STEWARD_ROLE`, `ROLES_FILE`, the resolved-role helpers (`roleK8sTargets`, `roleSentryTargets`, `roleOpsTargets`, `roleDepsTargets`, `rolePulls`, `rolePullWatches`, `roleOpsTeam`, `roleUsesOps`, `usesKind`, `legacyRole`) and the `check` problems (`watchProblems`, `pullWatchProblems`, `bindingRoles`, `projectStewardProblem`, `roleProblems`, `roleWarnings`, `filingTargetProblems`). `config.ts` keeps only the project-file schema primitives the roles module reads (`dependabotRepoSchema`, `filingTargetSchema`, `playbookName`, `LINEAR_PRIORITIES`, `sentrySlug`, `k8sTargets`, `sentryTargets`, `filingTargetOf`, `STEWARDS_DIR`, `PROJECT_FILE`, `zodMessage`). Moving the role helpers with the parsing keeps `roles.ts` a one-way consumer of `config.ts` (no import cycle); the call sites in `server.ts` and `config.test.ts` import from `roles.ts`. - **`ready-gate.ts`** — `briefGate` and `syncApprovalRequest` leave the `server.ts` closure, taking a `ReadyGateContext { store, specHashFor, specTextFor, specDiff, log }` (the `approvals.ts` pattern). It also now owns the ready gate's pure state moved from `reconcile.ts`: `gateState` (`GateState`), `approvalWanted`, and the checklist helpers `reportedItem`, `resolveChecklistItem` and `checklistKey`. It takes `specDiff` through the context, so it imports only a type from `reconcile.ts` and there is no runtime cycle; `reconcile.ts` keeps `gateKnownUnmet` and reads `gateState`/`reportedItem` from `ready-gate.ts`. - **`gate.ts`** — now owns the reviewer's marker and the approval words moved from `reconcile.ts`: `reviewMarker`, `latestReviewMarker`, `classifyApproval`, `productReason`, with `ReviewMarker`, `ApprovalVerdict` and `UNREADABLE_VERDICT`. `gate.ts` no longer imports from `reconcile.ts`; `reconcile.ts` imports the marker readers back, a one-way dependency. `gate.ts` is a `SELF_WIDENING_PATHS` file, so this stays owner-reviewed. - **`escalations.ts` (new)** — `routedTo`, `nextRoute` and `deliver` (now `deliverEscalation`) leave the `server.ts` closure, taking an `EscalationsContext { store, scheduleReconcile, wakeProject }`. - **`approvals.ts`** — the project steward asked for the approval-recording path to leave `server.ts` too, so a change to it is always the owner's. Moved here: `isProjectSteward`; `specApprovedByProject` (both spec ready-gate acceptance paths, the `approve: true` one and the project steward's approving `guidance` on a `resolve`); `placeReportedPull` (the `decideForPull` placement a steward reporting an `approve-pr`/`merge` item uses); `approveOwnerReviewedPull` (the `steward_update { approve }` recording path: the `isProjectSteward` check, the reviewer-escalated refusal, `decideFor`, `recordProjectDecision`, `deliverProjectApproval`); `declineOwnerReviewedPull` (the decline that resolves an `approve-decline` escalation); and `userSaid` and `userApprovedOnEntity` (the user's words in this thread, and their unsigned approving comment on the entity), plus `isUserThread`. `userSaid` keeps the whole filter — the `initiator` check, the plugin-sent hash, the lookback and the `sentSince` cutoff, in the original order with the early return — so only raw I/O is on the context: `turnRequests`, `pluginSent`, `approvalLookbackMs` and `sentSince`. `ApprovalsContext` also gains `loadDecisionsOrNull` (so the recording path can tell an unreadable project file from an empty decide list). `approvals.ts` is already in `SELF_WIDENING_PATHS`, so this stays owner-reviewed, and `SELF_WIDENING_PATHS` itself is unchanged. - **Untouched:** `decisions.ts`, `approvals-store.ts` (the rest of `SELF_WIDENING_PATHS`) and `source-contract.ts`. Scheduling (`syncScheduleRequest`), the approve-spec need and the forwarding handler stay in `server.ts`; PR 2 decides their lane. The modules that hold the moved gate and approval code are `plugins/stewards/gate.ts` and `plugins/stewards/approvals.ts`, both already named by `SELF_WIDENING_PATHS`, and `plugins/stewards/ready-gate.ts`, which the narrowed user entry names; PR 2 does not need a new path for any of them. `reconcile.ts` stays under the project entry in PR 2. `config.ts` gains three exports the roles module reads (`sentrySlug`, `playbookName`, `zodMessage`) and drops the role exports; `roles.ts` re-exports nothing. ## Testing - `npm run typecheck`: pass. - `npm test`: pass (32 files, 1496 tests; not all in the plugins CI matrix). - The moved functions are pinned to their modules by tests: `reconcile.test.ts` asserts `gate.ts` exports `reviewMarker`/`latestReviewMarker`/`classifyApproval`/`productReason` and `ready-gate.ts` exports `gateState`/`approvalWanted`/`reportedItem`/`resolveChecklistItem` while `reconcile.ts` exports none of them; `approvals.test.ts` pins `approvals.ts` and `approvals-store.ts` as self-widening, exercises `userSaid`'s own filtering (the plugin wake, a steward's turn, the `sentSince` cutoff and the lookback) and `isUserThread` on the store, so the logic fails the test if it moves back to `server.ts`. - `bb stewards gate` on three recent merged PRs before and after the refactor: identical (below). ### `bb stewards gate` before (on the pre-refactor server) ``` $ bb stewards gate #311 #311 — BB-280 head 6686c57; lane owner-reviewed reviewer: round 1 at 3fd262d (approved) decide list: "The stewards plugin's roles parsing, ready gate and approval and escalation plumbing outside the hard-coded self-widening list" (user) escalation: owner — the reviewer escalated it Decide list: 1 entry matched: - "The stewards plugin's roles parsing, ready gate and approval and escalation plumbing outside the hard-coded self-widening list" [user] — plugins/stewards/server.ts Deciding entry: "The stewards plugin's roles parsing, ready gate and approval and escalation plumbing outside the hard-coded self-widening list" [user]. Rule: a `user` entry wins; self-widening is always the user's; no match is the user's approval: owner-comment (2026-11-10T03:30:45Z) checks: success (green) open items: none gate met: yes $ bb stewards gate #310 #310 — BB-280 head f9a3442; lane no-review reviewer: round 1 at f9a3442 (approved) decide list: "A new dependency" (project) escalation: none Decide list: 1 entry matched: - "A new dependency" [project] — plugins/review-watch/package-lock.json Deciding entry: "A new dependency" [project]. Rule: a `user` entry wins; self-widening is always the user's; no match is the user's approval: owner-comment (2026-11-10T03:21:50Z) checks: success (green) open items: none gate met: yes $ bb stewards gate #309 #309 — BB-280 head 3faccc9; lane owner-reviewed reviewer: round 2 at 3faccc9 (approved) decide list: "The stewards plugin's roles parsing, ready gate and approval and escalation plumbing outside the hard-coded self-widening list" (user) escalation: owner — the reviewer escalated it Decide list: 1 entry matched: - "The stewards plugin's roles parsing, ready gate and approval and escalation plumbing outside the hard-coded self-widening list" [user] — plugins/stewards/server.ts Deciding entry: "The stewards plugin's roles parsing, ready gate and approval and escalation plumbing outside the hard-coded self-widening list" [user]. Rule: a `user` entry wins; self-widening is always the user's; no match is the user's approval: owner-comment (2026-11-10T03:31:08Z) checks: success (green) open items: none gate met: yes ``` ### `bb stewards gate` after (identical) `bb stewards gate` runs the server's deployed `bb-server` image, not this branch, so the "after" runs below are the same command from the same author thread against the same deployed plugin and are identical by construction. The refactor's real verification is the full `npm test` suite, which exercises the moved roles parsing (`config.test.ts` and `server.test.ts`), the ready gate and the escalation routing through the plugin (`server.test.ts`), plus the reviewer's line-by-line comparison of the moved code. Once PR 2 and a `bb-server` build carry this refactor, the same command will exercise the moved code on a server. ``` $ bb stewards gate #311 #311 — BB-280 head 6686c57; lane owner-reviewed reviewer: round 1 at 3fd262d (approved) decide list: "The stewards plugin's roles parsing, ready gate and approval and escalation plumbing outside the hard-coded self-widening list" (user) escalation: owner — the reviewer escalated it Decide list: 1 entry matched: - "The stewards plugin's roles parsing, ready gate and approval and escalation plumbing outside the hard-coded self-widening list" [user] — plugins/stewards/server.ts Deciding entry: "The stewards plugin's roles parsing, ready gate and approval and escalation plumbing outside the hard-coded self-widening list" [user]. Rule: a `user` entry wins; self-widening is always the user's; no match is the user's approval: owner-comment (2026-11-10T03:30:45Z) checks: success (green) open items: none gate met: yes $ bb stewards gate #310 #310 — BB-280 head f9a3442; lane no-review reviewer: round 1 at f9a3442 (approved) decide list: "A new dependency" (project) escalation: none Decide list: 1 entry matched: - "A new dependency" [project] — plugins/review-watch/package-lock.json Deciding entry: "A new dependency" [project]. Rule: a `user` entry wins; self-widening is always the user's; no match is the user's approval: owner-comment (2026-11-10T03:21:50Z) checks: success (green) open items: none gate met: yes $ bb stewards gate #309 #309 — BB-280 head 3faccc9; lane owner-reviewed reviewer: round 2 at 3faccc9 (approved) decide list: "The stewards plugin's roles parsing, ready gate and approval and escalation plumbing outside the hard-coded self-widening list" (user) escalation: owner — the reviewer escalated it Decide list: 1 entry matched: - "The stewards plugin's roles parsing, ready gate and approval and escalation plumbing outside the hard-coded self-widening list" [user] — plugins/stewards/server.ts Deciding entry: "The stewards plugin's roles parsing, ready gate and approval and escalation plumbing outside the hard-coded self-widening list" [user]. Rule: a `user` entry wins; self-widening is always the user's; no match is the user's approval: owner-comment (2026-11-10T03:31:08Z) checks: success (green) open items: none gate met: yes ```
Implements BB-283.
bb stewards needs --allcalledneedViewon closed rows too, so a doneapprove-prrow on a merged PR printed[not actionable: …]even though it is a record of what happened, not an open item. The--allbranch now skipsneedViewfor astatus === "done"row and prints its resolution as→ <resolution>(nothing when null); open rows and--jsonoutput are unchanged, andneedViewitself is untouched.Classification
owner-reviewed: the diff touches
bb-plugins/plugins/stewards/server.ts, matched by theuserentry "The stewards plugin's roles parsing, ready gate and approval and escalation plumbing outside the hard-coded self-widening list", so the owner approves.What changes
plugins/stewards/server.ts: in the--allbranch ofneeds, a done row prints→ <resolution>(nothing when the resolution is null) and never a[not actionable: …]suffix; an open row keeps the current line byte-for-byte, andneedViewis unchanged.plugins/stewards/server.test.ts: a doneapprove-prneed on a merged, observed PR shows→ Approved on the Stewards page.and no[not actionable; an open need's--allline is asserted byte-for-byte.Testing
cd plugins/stewards && npm run typecheck: pass.npm test: pass (1478 tests, 31 files;stewardsis in the CI matrix).[not actionable: no reviewer round yet]) and passes with the change.Stacking
Based on
bb/bb-268-free-text-answer-must-carry-text-thr_p26s3p7mzt(#309). Merge that first, then retarget this tomain.