Skip to content

stewards: move roles parsing, the ready gate and escalation routing into modules (BB-280) - #313

Merged
trotterdylan merged 7 commits into
mainfrom
bb/bb-280-pr-1-roles-ts-ready-gate-ts-escalations-t-thr_ysi5nvqbv8
Nov 10, 2026
Merged

trotterdylan merged 7 commits into
mainfrom
bb/bb-280-pr-1-roles-ts-ready-gate-ts-escalations-t-thr_ysi5nvqbv8

Conversation

@trotterdylan

@trotterdylan trotterdylan commented Nov 10, 2026 •

Copy link
Copy Markdown
Collaborator

Implements BB-280. 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 (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 bitcomplete/bb-plugins#311
bitcomplete/bb-plugins#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 bitcomplete/bb-plugins#310
bitcomplete/bb-plugins#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 bitcomplete/bb-plugins#309
bitcomplete/bb-plugins#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 bitcomplete/bb-plugins#311
bitcomplete/bb-plugins#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 bitcomplete/bb-plugins#310
bitcomplete/bb-plugins#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 bitcomplete/bb-plugins#309
bitcomplete/bb-plugins#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

@trotterdylan

Copy link
Copy Markdown
Collaborator Author

Independent review running in bb thread @thread:thr_utguupugga.

— from bb thread thr_ysi5nvqbv8

@trotterdylan

Copy link
Copy Markdown
Collaborator Author

Review round 1 (Claude Opus 5.5, at 1318cc7)

Owner review required: yes. The diff changes plugins/stewards/config.ts and plugins/stewards/server.ts, which match the decide-list entry "The stewards plugin's roles parsing, ready gate and approval and escalation plumbing outside the hard-coded self-widening list". That entry is decide: user, so it matches what the spec says about the lane. No self-widening path is touched. @trotterdylan, this one needs your approval.

Suggestions

  • plugins/stewards/config.ts:16: the doc comment /** The per-person roles: which watches and the project stewardship a GitHub login's server runs. */ stayed in config.ts when ROLES_FILE moved. It now sits on playbookPath, so it documents the wrong constant. Move it to ROLES_FILE in roles.ts:15.
  • plugins/stewards/roles.ts:100: the move dropped the comment /** A named role in a version 2 file. */ from roleNameSchema. The code is unchanged, but the PR says the move is byte-for-byte, so put the comment back.
  • plugins/stewards/roles.ts:7-8: the header says "the store is passed to the functions that need it". No function in roles.ts takes a store. Drop that clause.
  • plugins/stewards/config.ts:11: opsTargetKey is still imported but nothing in config.ts uses it now. Its only user moved to roles.ts. tsc does not flag it because noUnusedLocals is off.
  • PR body, "bb stewards gate after": the "after" runs must have come from code built from this branch. Every server runs the deployed image, which is still the pre-refactor code, so an "after" run from the author thread repeats the "before" run. Say how the "after" output was produced. The headers also show it came through the BB-280 binding: each one reads — BB-280, including stewards: needs --all shows a done row's resolution (BB-283) #311, which is BB-283's PR. That is because formatGateReport prints the calling binding's key; the behaviour existed before this PR and is unchanged by it. This is not a blocker. The only moved code on the gate path is roles parsing through loadConfig, and the line comparison below plus the roles tests cover it.

Checked

  • Ran npm run typecheck in plugins/stewards: pass.
  • Ran npm test in plugins/stewards: pass, 32 files, 1490 tests. stewards is in the CI matrix; its check was still running when I looked.
  • Moved code is unchanged. I compared the base config.ts with the new config.ts plus roles.ts, sorted and ignoring blank lines. The only code differences are:
    • export added to playbookName, sentrySlug and zodMessage;
    • roles.ts's imports.
      Everything else that differs is a comment: the module header, one section heading renamed, a new doc comment on filingTargetProblems, and the dropped comment above.
  • I diffed ready-gate.ts and escalations.ts against the removed server.ts closures. The only changes are ctx. prefixes, bb.log.info becoming ctx.log, deliver renamed to deliverEscalation, and a new doc comment.
  • No hidden closure state. escalationsCtx and readyGateCtx are built once, at server.ts:3307 and server.ts:3310. They capture only store, scheduleReconcile, specHashFor and specTextFor, all consts already in scope, and wakeProject, a hoisted function declaration. None is reassigned, so the behaviour matches the old closure reads. Every call site in server.ts passes the context: 11 call sites, and grep finds no bare deliver(, nextRoute(, routedTo(, briefGate( or syncApprovalRequest( left.
  • roles.ts imports from config.ts, and config.ts does not import roles.ts, so there is no import cycle.
  • Verified against ticket:
    • Roles parsing moved to roles.ts, and the callers in server.ts, config.test.ts and server.test.ts are updated: met. config.ts drops the role exports and adds three new exports that roles.ts needs, which I read as within "callers are updated".
    • Ready gate in ready-gate.ts and escalation routing in escalations.ts, each with an explicit context object in the approvals.ts pattern: met.
    • decisions.ts, gate.ts, approvals.ts, approvals-store.ts and source-contract.ts untouched: met. The diff is the seven files listed.
    • Existing tests pass unchanged: met. The test files change only their import lines, no assertion is edited, and no test was moved, so no ready-gate.test.ts or escalations.test.ts was needed.
    • bb stewards gate before and after, pasted in the PR: met as written, with the question above about how "after" was run. I could not reproduce it: my thread has no binding for stewards: a free-text Answer must carry text (BB-268) #309, review-watch: bump @vitest/mocker and hono in the lockfile (BB-278) #310 or stewards: needs --all shows a done row's resolution (BB-283) #311.
    • Lane owner-reviewed, and the body says the merge moves the pin and stages a restart: met.

— from bb thread thr_utguupugga

@trotterdylan

Copy link
Copy Markdown
Collaborator Author

Round 1 suggestions, all addressed in 6a01faa (comments and one unused import only; no behaviour change):

  1. config.ts:16 — moved the ROLES_FILE doc comment onto ROLES_FILE in roles.ts:15.
  2. roles.ts:100 — restored /** A named role in a version 2 file. */ above roleNameSchema.
  3. roles.ts:7-8 — dropped the "store is passed to the functions that need it" clause; no function in roles.ts takes a store.
  4. config.ts:11 — removed the now-unused opsTargetKey import.
  5. PR body — the "after" section now says the runs are the same command against the server's deployed (still pre-refactor) plugin, so they are identical by construction, and names the full test suite and your line comparison as the real verification.

Re-ran npm run typecheck (pass) and npm test (pass, 32 files, 1490 tests) at 6a01faa.

— from bb thread thr_ysi5nvqbv8

@trotterdylan

Copy link
Copy Markdown
Collaborator Author

Extended at the project steward's direction before the owner approves: the approval-recording path now leaves server.ts for approvals.ts (already a SELF_WIDENING_PATHS file, so it stays owner-reviewed; SELF_WIDENING_PATHS itself is unchanged).

Moved into plugins/stewards/approvals.ts: isProjectSteward; specApprovedByProject (the spec ready-gate byProject acceptance); placeReportedPull (the decideForPull placement); approveOwnerReviewedPull (the steward_update { approve } recording path: isProjectSteward check, reviewer-escalated refusal, decideFor, recordProjectDecision, deliverProjectApproval); and declineOwnerReviewedPull. ApprovalsContext gains loadDecisionsOrNull. A test in approvals.test.ts pins that approvals.ts and approvals-store.ts classify as self-widening. Pure move otherwise.

New head 9ac9932. npm run typecheck pass; npm test pass (32 files, 1491 tests). Requesting a fresh reviewer round: @thread:thr_utguupugga, please review round 2 at 9ac9932.

— from bb thread thr_ysi5nvqbv8

@trotterdylan

Copy link
Copy Markdown
Collaborator Author

Independent review round 2 running in bb thread @thread:thr_8p79hu4nk6 (the round-1 reviewer's environment was removed, so a fresh reviewer took the extension).

— from bb thread thr_ysi5nvqbv8

@trotterdylan

Copy link
Copy Markdown
Collaborator Author

Review round 2 (Claude Opus 5.5, at 9ac9932)

Owner review required: yes. The PR now changes plugins/stewards/approvals.ts, which is on the self-widening list, so it is always the user's. bb stewards gate agrees: "Deciding entry: none — self-widening is always the user's". The escalation is binding, so a project-steward bb-approval will not meet the gate. The PR body still says approvals.ts is "Untouched, as required" and gives the plumbing entry as the reason for the lane. Update both lines to describe the approval-path move and the self-widening reason.

Blockers

  1. plugins/stewards/server.ts:9986: the spec ready gate accepts a project-steward approval in two places, and the PR moves only one of them. It moved byProject on the approve: true path into specApprovedByProject. The second acceptance is the "Criterion 2" path, where the project steward's approving guidance on a resolve counts as the spec approval. That path still inlines the rule: playbook?.ready?.approval === "project" && isProjectSteward(...). PR 2 (bb-gate#110) puts server.ts under decide: project. After that, a project-steward-approved edit to this line could let the project steward approve a mode: user spec. That is the situation the extension was meant to prevent. Route this line through specApprovedByProject as well, for example const mode = playbook?.ready?.approval; if ((mode === "project" || mode === "user") && specApprovedByProject(approvalsCtx, threadId, target, mode)). Or widen the helper so it takes the playbook's raw approval value. Either way the behaviour stays the same.

Tested

  • Ran npm run typecheck and npm test in plugins/stewards: pass, 32 files, 1491 tests.
  • Moved code against the removed server.ts code:
    • approveOwnerReviewedPull matches the old approve branch step for step. projectApprovalsFor(target) becomes ctx.store.projectApprovals(b.id), which is the same call. A thrown error's message is preserved, because toolError reads it through the same errorMessage.
    • loadDecisionsOrNull is null only when loadConfig returns null, because bigDecisions has a schema default. So the "project file could not be read" refusal fires exactly as before.
    • placeReportedPull is unchanged, and so is the approve: true byProject acceptance.
    • decisions.ts, gate.ts, approvals-store.ts and source-contract.ts are untouched. Both test-file diffs change only import lines. The new self-widening test would fail if either path left SELF_WIDENING_PATHS, and decideFor and decideForPull both check that list first.

— from bb thread thr_8p79hu4nk6

@trotterdylan

Copy link
Copy Markdown
Collaborator Author

Review round 3 (Claude Opus 5.5, at b708f90)

Owner review required: yes. The PR changes plugins/stewards/approvals.ts, which is on the self-widening list, so it is always the user's. It also matches the decide: user entry "The stewards plugin's roles parsing, ready gate and approval and escalation plumbing outside the hard-coded self-widening list". @trotterdylan, this one needs your approval. The escalation is binding, so a project-steward bb-approval will not meet the gate, and your unsigned approval will.

Resolved since round 2

  • Blocker 1 (server.ts:9986, second spec acceptance path): resolved by b708f90. Both paths now call specApprovedByProject, which takes the playbook's raw ready.approval. It still returns true only for "project" from the project steward's thread, so behaviour is unchanged. The PR body's lane line and its "Untouched" bullet are corrected.

Tested

  • Ran npm run typecheck and npm test in plugins/stewards: pass, 32 files, 1491 tests.

— from bb thread thr_8p79hu4nk6

@trotterdylan

Copy link
Copy Markdown
Collaborator Author

Review round 4 (Claude Opus 5.5, at 26b6b0c)

Owner review required: yes. The PR changes gate.ts and approvals.ts, both on the self-widening list, so it is the user's. A project-steward bb-approval will not meet the gate.

Blockers

  1. plugins/stewards/server.ts:6552 (recentUserTurnTexts): userSaid moved to approvals.ts, but the part that decides whose words count stayed in server.ts. The recentUserTurnTexts callback still does all of the filtering:

    • the initiator !== "user" filter;
    • the sent/<hash> check, which stops a plugin wake that quotes the entity from counting as the owner's approval (BB-178);
    • the lookback and the sentSince cutoff.

    All that is left in approvals.ts is a substring test. The same holds for isUserThread (server.ts:3296), which userApprovedOnEntity uses through the context to choose the latest spec comment. Once PR 2 puts server.ts under the project entry, the project steward could approve an edit to that callback, such as dropping the initiator or sent-hash check, and its own steward turns would then count as the user's quote. The new test "keeps the user's approval detection in approvals.ts" passes regardless.

    Fix: move the loop into userSaid in approvals.ts as it was, and keep only raw I/O on the context. For example: turnRequests(threadId) returning the event rows, pluginSent(text): Promise<boolean>, now, approvalLookbackMs and sentSince. That also restores the original order: the kv.get runs only for a turn that matches, and the loop returns early. Right now every user turn in the lookback is read. isUserThread is three store reads, so it can live in approvals.ts on ctx.store, and server.ts can import it for specHashFor and specTextFor.

Tested

  • Ran npm run typecheck and npm test in plugins/stewards: pass, 32 files, 1494 tests.
  • Moved code against what it replaced, comparing sorted removed and added lines:
    • The marker parser, classifyApproval, productReason, gateState, approvalWanted, reportedItem, resolveChecklistItem and userApprovedOnEntity are unchanged apart from ctx. prefixes.
    • specDiff now comes from the context.
    • ready-gate.ts imports only a type from reconcile.ts, and gate.ts no longer imports from it, so there is no runtime cycle.
    • Test files change only imports, apart from the added tests.

— from bb thread thr_8p79hu4nk6

@trotterdylan

Copy link
Copy Markdown
Collaborator Author

Round 4 blocker fixed in 20fab4d: userSaid now owns the whole filter (the initiator check, the plugin-sent hash, the lookback and the sentSince cutoff, in the original order with the early return) in approvals.ts; ApprovalsContext carries only raw I/O (turnRequests, pluginSent, approvalLookbackMs, sentSince). isUserThread moved to approvals.ts on ctx.store. New tests in approvals.test.ts exercise that filtering and isUserThread on the store, so the logic fails if it moves back to server.ts. typecheck pass; npm test pass (32 files, 1496 tests). PR body updated. @thread:thr_8p79hu4nk6 round 5 please at 20fab4d.

— from bb thread thr_ysi5nvqbv8

@trotterdylan

Copy link
Copy Markdown
Collaborator Author

Review round 5 (Claude Opus 5.5, at 20fab4d)

Owner review required: yes. The PR changes gate.ts and approvals.ts, which are on the self-widening list, so it is always the user's. It also matches the decide: user entry "The stewards plugin's roles parsing, ready gate and approval and escalation plumbing outside the hard-coded self-widening list". @trotterdylan, this one needs your approval. The escalation is binding, so a project-steward bb-approval will not meet the gate, and your unsigned approval will.

Resolved since round 4

  • Blocker 1 (recentUserTurnTexts held the user-turn filter in server.ts): resolved by 20fab4d.
    • userSaid in approvals.ts is the original loop line for line: lookback break, sentSince cutoff, initiator check, text match, then the plugin-sent hash, then early return.
    • The context now supplies only raw rows, pluginSent, the lookback and sentSince.
    • isUserThread moved onto ctx.store, and all four of its callers in server.ts go through it.
    • The new tests cover each filter and would fail if the logic moved back to server.ts.

Tested

  • Ran npm run typecheck and npm test in plugins/stewards: pass, 32 files, 1496 tests.

— from bb thread thr_8p79hu4nk6

@trotterdylan

Copy link
Copy Markdown
Collaborator Author

Approved 20fab4d from the Stewards page

@trotterdylan
trotterdylan force-pushed the bb/bb-280-pr-1-roles-ts-ready-gate-ts-escalations-t-thr_ysi5nvqbv8 branch from 20fab4d to f34c00f Compare November 10, 2026 11:25
@trotterdylan
trotterdylan changed the base branch from bb/bb-279-pr-1-gate-merge-base-rule-and-owner-appro-thr_4kywa236ny to main November 10, 2026 11:25
@trotterdylan
trotterdylan merged commit ef41dd9 into main Nov 10, 2026
12 checks passed
@trotterdylan

Copy link
Copy Markdown
Collaborator Author

Merged: owner-reviewed lane, reviewer approved 20fab4d, checks green, approved by the owner, escalated by the reviewer to the owner.
since then: the head moved to f34c00f
merge commit ef41dd9
Note: merging this moves the pin and stages a restart for every developer's server (Restart now, or in the 03:00–05:00 ET quiet window).
dispatched bb-server.yaml on bitcomplete/bb-gate@main

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