Skip to content

stewards: needs --all shows a done row's resolution (BB-283) - #311

Merged
trotterdylan merged 1 commit into
mainfrom
bb/bb-283-needs-all-resolution-done-rows-thr_jnbk7xzhny
Nov 10, 2026
Merged

trotterdylan merged 1 commit into
mainfrom
bb/bb-283-needs-all-resolution-done-rows-thr_jnbk7xzhny

Conversation

@trotterdylan

Copy link
Copy Markdown
Collaborator

Implements BB-283. bb stewards needs --all called needView on closed rows too, so a done approve-pr row on a merged PR printed [not actionable: …] even though it is a record of what happened, not an open item. The --all branch now skips needView for a status === "done" row and prints its resolution as → <resolution> (nothing when null); open rows and --json output are unchanged, and needView itself is untouched.

Merging this moves the bb-plugins pin, builds a new bb-server image and stages a restart for every developer's server. Each developer picks the moment at Restart now, or it lands in the quiet window (03:00-05:00 US Eastern).

Classification

owner-reviewed: the diff touches bb-plugins/plugins/stewards/server.ts, matched by the user entry "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 --all branch of needs, 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, and needView is unchanged.
  • plugins/stewards/server.test.ts: a done approve-pr need on a merged, observed PR shows → Approved on the Stewards page. and no [not actionable; an open need's --all line is asserted byte-for-byte.

Testing

  • cd plugins/stewards && npm run typecheck: pass.
  • npm test: pass (1478 tests, 31 files; stewards is in the CI matrix).
  • Verified the done-row test fails against the unmodified branch head ([not actionable: no reviewer round yet]) and passes with the change.
  • Not tested: nothing beyond the CLI's text output.

Stacking

Based on bb/bb-268-free-text-answer-must-carry-text-thr_p26s3p7mzt (#309). Merge that first, then retarget this to main.

@trotterdylan

Copy link
Copy Markdown
Collaborator Author

Independent review running in bb thread @thread:thr_56djnwd3en.

— from bb thread thr_jnbk7xzhny

@trotterdylan

Copy link
Copy Markdown
Collaborator Author

Review round 1 (glm-5p3, at 3fd262d)

Owner review required: yes — the diff touches plugins/stewards/server.ts, matched by 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), and it changes what the existing needs --all command prints on done rows; the change is also substantial with no approved spec on BB-283 (product). @trotterdylan, this one needs your approval — binding: a project-steward bb-approval will not meet the gate; your unsigned approval will.

The done-row line is exactly the open-row head plus → <resolution> (nothing when null), so open rows are byte-for-byte unchanged, --json (which returns before this map) is untouched, and needView is unchanged — each criterion's test is present, and I confirmed the done-row test fails on the base branch with [not actionable: no reviewer round yet]. Scope stays inside the --all branch and server.test.ts.

Tested

  • Ran: cd plugins/stewards && npm run typecheck && npm test (pass; 1478 tests, 31 files; in the CI matrix).

Looks good.

— from bb thread thr_56djnwd3en

@trotterdylan

Copy link
Copy Markdown
Collaborator Author

Approved 3fd262d from the Stewards page

@trotterdylan
trotterdylan force-pushed the bb/bb-283-needs-all-resolution-done-rows-thr_jnbk7xzhny branch from 3fd262d to 6686c57 Compare November 10, 2026 03:31
@trotterdylan
trotterdylan changed the base branch from bb/bb-268-free-text-answer-must-carry-text-thr_p26s3p7mzt to main November 10, 2026 03:31
@trotterdylan
trotterdylan merged commit ed689ed into main Nov 10, 2026
24 checks passed
@trotterdylan
trotterdylan deleted the bb/bb-283-needs-all-resolution-done-rows-thr_jnbk7xzhny branch November 10, 2026 03:34
@trotterdylan

Copy link
Copy Markdown
Collaborator Author

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 bb-plugins pin and stages a restart for every developer's server (Restart now, or in the 03:00–05:00 ET quiet window).

_— from bb thread

trotterdylan added a commit that referenced this pull request Nov 10, 2026
…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
```
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