[trotterdylan]

What changes

A Dependabot bump that cannot merge as is comes back as a worker's own replacement PR (bb-plugins#92 → #93 on 2026-10-26). The deps-prs binding stayed on the closed bot PR: the gate never looked at the replacement, no Merge item was filed for it (the plugin never adopts a PR the signed-in login wrote), and the steward's only ask was a decision escalation whose "Merge" button recorded an answer for a steward that may not merge.

• steward_report { replacedBy: "owner/repo#N" } (pull-request stewards): the binding moves to the replacement. Same id, thread, playbook, guidance, escalations and workers; entity, snapshot and phase become the replacement's; approve-pr/merge items and approval escalations filed for the old PR are withdrawn; an ending begun by the old PR's close is undone; the thread is retitled and the source leases refreshed; a reconcile is scheduled so the ordinary gate files the replacement's Merge item. Refused unless the replacement is an open PR in the same repository with no steward of its own.
• entityId becomes a patchable binding column.
• The deps-prs playbook template: every bump gets the pr-reviewer round, the Merge item on the Stewards page is the ask (no more decision escalation for the merge), and a replacement is reported with replacedBy.
• README, DESIGN.md §19 and the stewards skill describe it.

The project's .bb copy of the playbook, AGENTS.md and the pr-reviewer skill have already landed on .bb master (3dbf032). Until this merges and the image rolls, a steward on the current image reads the new playbook and may send replacedBy, which the current build's non-strict schema strips: it gets "Reported." back and the binding still ends on the bot PR's close. Merging and deploying promptly closes that window.

Classification

Owner-reviewed: it changes what the deps-prs flow does for an existing kind of PR (a Merge item replaces the decision escalation).

Testing

npm run typecheck and npm test in plugins/stewards: 542 tests pass. New tests cover the rebind (binding id and thread kept, old key gone, leases follow, the old PR closing no longer ends it, the Merge item is filed on the replacement once its marker is approved on green checks, and a wrap-up report that closes the old PR and then reports replacedBy in the same turn undoes the ending it began) and the refusals (bad ref, other repo, self, unreadable, closed, already stewarded, and a binding whose own key cannot be read).

A merge to main rebuilds the bb-server image and stages a restart on every developer's server.

🤖 Generated with Claude Code

[trotterdylan]

Independent review running in bb thread @thread:thr_8avbxfdkh6.

— from bb thread thr_y5jcbn2baq

[trotterdylan]

Review round 1 (GLM 5.3, at 471dcb6)

Owner review required: yes — it changes what the deps-prs flow does for an existing kind of PR: the ask for a mergeable bump changes from a decision escalation to a Merge item, and the playbook template's guidance changes meaning. @trotterdylan, this one needs your approval. (No bigDecisions entry with decide: project matches this diff — store.ts adds no migration and no package.json is touched — so the lane is the user's, agreeing with the body's Classification line.)

Blockers

1. plugins/stewards/server.test.ts (depsBound tests) — the spec claims "an ending begun by the old PR's close is undone", and that is the primary real-world path, but no test exercises it. In the flow the new template describes, the steward's first chance to report replacedBy is the wrap-up wake after the worker closes the bot PR: the close doorbell reconciles, beginEnding sets ending: "terminal" and endingAt (but not endedAt, so ownBinding still admits the report), and the steward reports replacedBy during that turn. Both new tests bind and report while the old PR is still open, and the second half of the "moves" test only proves that a close after the rebind does not begin an ending. Please add a test that binds on the bot PR, closes it and rings so the ending begins, then reports replacedBy and asserts the ending is undone (ending/endingAt/followThrough null, binding open on the replacement, thread kept) and the gate still files the replacement's Merge item. As it stands the one claimed behaviour that makes the feature work in the common ordering is unverified — and it runs through the newest code (store.patch resetting ending/endingAt/followThrough/followThroughUntil).

Suggestions

• plugins/stewards/server.ts, steward_report execute (~4925) — the rebind commits before the rest of the input is validated: a report with { replacedBy, followThrough } (a natural combination in that wrap-up wake) moves the binding and then returns followThrough is only for after a terminal state, and the same ordering applies to the opsTickets/ready-checklist/blockedOn refusals. The steward recovers by reporting again, but the error is confusing in that exact turn ("acme/web#10 is not in one now" right after the steward watched the old PR close), and the retry must also drop replacedBy or it hits the self-refusal. Validating the other fields before the rebind — or judging followThrough against the pre-rebind ending — would make the wrap-up report one round trip.
• plugins/stewards/server.ts, replaceBinding (~4727) — when b.url and b.entityKey are both unparseable (own === null), the same-repo and self checks are skipped and store.byEntity can then return b itself, letting a binding "rebind to itself". A github pr binding's key is always owner/repo#N, so this is a defensive gap only; refusing when own === null would close it.
• Deployment sequencing — the body says the .bb guidance branch lands after this is deployed, but it has already landed on .bb master (3dbf032, "deps-prs: review every bump, merge from the Stewards page, follow a replacement PR"), and the workspace AGENTS.md already describes the new flow. Until this merges and the image rolls, a deps-prs steward on the current image reads the new playbook and may report replacedBy, which the old build's (non-strict) zod schema silently strips — it gets "Reported." back and the binding still ends on the bot PR's close. Nothing in this diff is wrong; merging and deploying promptly is the remedy, but the body's sequencing line is outdated and the merge comment should acknowledge the window.

Checked

• Ran npm run typecheck (clean) and npm test in plugins/stewards: 540 tests pass in 18 files, matching the body. stewards is in the plugins CI matrix (quality_checks (stewards) green on this head), so CI skipped nothing here; I ran the suite anyway.
• Verified against the spec: the move keeps id/thread/playbook/guidance/escalations/workers and swaps entity/snapshot/phase (tested); approve-pr/merge items and approval escalations for the old PR are withdrawn mechanically (closeNeedByKey, resolve(..., "plugin") — no guidance write, correct); the thread is retitled, leases refreshed via whole-set sendInterest, and the scheduled reconcile has the ordinary gate file the replacement's Merge item (tested: marker approved + green checks → one merge need); all six refusals are tested; entityId becoming patchable needs no migration and is safe under UNIQUE (project_id, source, entity_id) (the byEntity pre-check covers it, and a create-then-patch race throws into toolError, no corruption); no MIGRATIONS entry touched; no cross-repo contract change (source-contract.ts copies untouched, contract tests green); the deps-prs template, README, DESIGN.md §19 and the plugin's SKILL.md copy all describe the new flow, and the config/prompt tests match the shipped template text.

— from bb thread thr_8avbxfdkh6

[trotterdylan]

Round 1 addressed in f7752d8.

• Blocker 1 (missing wrap-up/ending-undo test): fixed in f7752d8. Added undoes an ending begun by the replaced pull request's close when the steward reports the replacement in that wrap-up turn: bind on the bot PR, close it and ring so beginEnding sets ending: "terminal"/endingAt, then report replacedBy in that turn and assert ending/endingAt/followThrough/followThroughUntil are null, endedAt is null, the binding is on thread-briefs: ask the summarizer for status instead of deriving it #10 with the same thread, and the replacement's approved marker + green checks still files its Merge item. Confirmed it fails on the pre-fix code at expect(reported.error).toBe(false).
• Suggestion 1 (validation order after the rebind): fixed in f7752d8. The opsTickets, choices and followThrough checks now run before replaceBinding, and followThrough is judged against the ending the report found (endingBefore), so a wrap-up report that carries replacedBy and followThrough lands in one round trip. The ready checklist is resolved against the same playbook (deps-prs has no ready gate) and blockedOn is deliberately still judged after the move, because the move withdraws the old PR's needs — the realistic combination was followThrough.
• Suggestion 2 (own === null guard): fixed in f7752d8. replaceBinding now refuses when its own key does not read as owner/repo#N, closing the rebind-to-itself path; added a test for the refusal.
• Suggestion 3 (deployment sequencing): fixed. The PR body's last paragraph now says the .bb guidance already landed on .bb master (3dbf032) and names the window until the image rolls; the merge comment will acknowledge it.

npm run typecheck clean and npm test in plugins/stewards: 542 tests pass (was 540; the two new tests). Checks are running on f7752d8.

— from bb thread thr_y5jcbn2baq

[trotterdylan]

Review round 2 (GLM 5.3, at f7752d8)

Owner review required: yes — it changes what the deps-prs flow does for an existing kind of PR (a Merge item replaces the decision escalation). @trotterdylan, this one needs your approval; my verdict below is only the reviewer's half of the gate.

Resolved since round 1

• Blocker 1 (no test for an ending begun by the old PR's close): resolved by f7752d8. The new test closes the bot PR, rings so beginEnding runs (asserted: ending: "terminal", endingAt set, endedAt null), then reports replacedBy — with followThrough, the natural wrap-up combination — in that same turn, and asserts the ending is undone (all four ending/follow-through fields null, binding open on the replacement, thread kept) and that the replacement's Merge item is filed once its marker is approved on green checks. This is the realistic main path and it now fails without the fix.
• Suggestion (validation after the rebind committed): resolved — opsTickets/choices/followThrough are validated before the rebind, and followThrough is judged against the ending the report found, so the wrap-up report lands in one round trip. Checked the accepted-but-not-stored followThrough in that combination: deliberate and coherent — after the move there is no terminal ending to follow through from, the test asserts it stays null, and the reply describes the ordinary open-binding behaviour ("woken when something actionable changes"), so the steward is not told a follow-through is active.
• Suggestion (own === null defensive gap): resolved — an unparseable own key now refuses with a clear error, with a test.
• Suggestion (deployment-sequencing line outdated): resolved — the PR body now states the .bb guidance already landed on master (3dbf032) and names the window.

Checked

• Reviewed the round-2 delta (validation reorder, endingBefore, the own === null refusal, two new tests) with the round 1 questions: no new issues; the reordered refusals all fire before any state changes, priorReady/priorSnapshot still read the moved binding exactly as before, and an ops steward reporting replacedBy is still refused by the kind check.
• Ran npm run typecheck (clean) and npm test in plugins/stewards: 542 tests pass in 18 files, matching the body. stewards is in the CI matrix; nothing was skipped.
• Lane unchanged: the merge still changes the live deps-prs flow, so it stays owner-reviewed; no bigDecisions entry with decide: project matches the diff.

— from bb thread thr_8avbxfdkh6

[trotterdylan]

Approved f7752d8 from the Stewards page

[trotterdylan]

Merged: owner-reviewed, approved by trotterdylan on f7752d8 (comment 2026-10-26T18:36:05Z); reviewer approved f7752d8, checks green; merge commit 01c39d3.

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

Window noted (round-1 suggestion 3): the .bb guidance is already on master (3dbf032), so a deps-prs steward on the current image may read the new playbook and send replacedBy, which the current build's non-strict schema strips until the new image rolls. Merging and deploying promptly closes it.

— from bb thread thr_y5jcbn2baq