Skip to content

stewards: re-match the deployment rule for every non-terminal watch (BB-337) - #369

Merged
trotterdylan merged 1 commit into
mainfrom
bb/bb-337-re-match-deployment-rule-on-every-look-bo-thr_kfgn64rvhd
Nov 14, 2026
Merged

trotterdylan merged 1 commit into
mainfrom
bb/bb-337-re-match-deployment-rule-on-every-look-bo-thr_kfgn64rvhd

Conversation

@trotterdylan

@trotterdylan trotterdylan commented Nov 14, 2026 •

Copy link
Copy Markdown
Collaborator

Implements BB-337. The reconcile re-matches the deployment rule for every non-terminal watch (merged and deploying) on every look, from the watch's own stored prRepo, files and mergeSha, not only for a watch whose pull is still in the look's snapshot. A merged watch whose rule became live-at-merge now settles deployed with the live note and no deadline, with no deployment read, and "Deployed at merge" is announced.

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

Was stacked on #368 (BB-333). #368 has merged, so this is rebased onto main and retargeted to it.

Classification

owner-reviewed: plugins/stewards/server.ts is the plumbing entry the project's user decide-list entry names.

What changes

  • server.ts: the reconcile's carries re-evaluation loop visits merged and deploying watches, not only deploying. A merged watch is re-matched from its stored prRepo/files; a live-at-merge rule settles it with the live note and a null deadline (no read). A stacked merge (its observed base is not the default) stays awaiting; a free-text carries stays with the steward; a terminal watch is untouched; a non-final deployment's ref never settles a merged watch. The settlement is announced as "Deployed at merge".
  • deploy.test.ts / server.test.ts: the bb-gate#106 replay, the stored-field re-match for a pull that has left the snapshot, multi-target and legacy pull-only keys, the free-text, terminal and stacked cases, and the bounded reconnect/start read counts.

This is a behaviour change, not a refactor. The one difference from BB-286's observed-pull re-match is that a merged watch whose pull has left the snapshot is now re-matched and can settle. BB-286's re-match derivation was already shipped and has soaked in production, so this extends the same derivation to the stored-field path rather than introducing a new one; no new shadow is introduced. No store migration; nothing is retired. contract.test.ts is untouched.

Testing

  • cd bb-plugins/plugins/stewards && npm run typecheck && npm test: pass (1908 tests, 40 files; the full suite, which CI's fixed matrix skips).
  • Reads/calls: the pinned fixture in read-counts.test.ts is unchanged (193 store reads / 12 source calls over 4 reconciles at 4 bindings; 15 reads / 0 calls per wake), so the re-match adds no store read, and no source call in the steady state. Against the production baseline (18.8 reads / 1.4 calls per reconcile at 26 bindings; 16.2 reads / 0 calls per wake), this change adds nothing in steady state. Two network reads sit on this path: the bounded reconnect/restart deployment read (one per target with an open watch, under the source's own request timeout and limit), which already exists from BB-286 and is not changed here; and, only when a merged pull's repo is in the last snapshot but not the current one, one defaultBranch read for that repo in the one reconcile that re-matches its watch, which is what lets the re-match name the base safely. Neither is a store read, and both are bounded.
  • Not tested: a live production soak of the unobserved-pull case; it is a rare shape and the unit and server tests cover it.

Revision

A **Revision:** for BB-291's "Intended gate differences" is proposed to the BB-337 steward (relayed to BB-291) before this merges: a merged watch whose pull has left the snapshot and whose rule is now live-at-merge settles deployed instead of staying merged; no merge-gate result changes.

@trotterdylan
trotterdylan force-pushed the bb/bb-337-re-match-deployment-rule-on-every-look-bo-thr_kfgn64rvhd branch 2 times, most recently from b8bcbdb to 8507926 Compare November 14, 2026 23:21
@trotterdylan
trotterdylan changed the base branch from bb/bb-333-pr-2-of-3-shadow-derivation-of-liveness-i-thr_za23kmc77j to main November 14, 2026 23:21
@trotterdylan

Copy link
Copy Markdown
Collaborator Author

Independent review running in bb thread @thread:thr_be2gfcwu8d.

— from bb thread thr_kfgn64rvhd

@trotterdylan

Copy link
Copy Markdown
Collaborator Author

Review round 1 (Claude Opus 5.5, at 8507926)

Owner review required: yes. It changes what an existing feature does: a merged watch can now settle deployed from the reconcile loop. Decide-list entry "The stewards plugin outside its authority seams (the app and sidebar UI, its skills, every other stewards file); …" is decide: project, so the project steward approves. (substantial=no: the BB-337 ready gate judged the ticket not substantial, and the diff is about 35 lines extending an existing loop.)

Blockers

  1. plugins/stewards/server.ts:5002: the stacked-merge guard fails open in exactly the case this PR adds. When the observed pull is found, recordMerged (BB-286) has already settled it or deliberately left it awaiting. So the new branch only changes the outcome when the pull is not in the snapshot, and in that case the guard is skipped and the watch settles. The watch stores no base, so a stacked merge cannot be told apart from a default-branch one. BB-260's rule is the opposite: "a base the read could not name is not treated as default."
    One transient read is enough to trigger it, because linkedPulls drops a pull whose snapshot throws or returns null. I reproduced it on this head with a probe test:

    • The ticket links acme/web#7, which is merged into feature-base with .bb/AGENTS.md under a live-at-merge rule. The first reconcile leaves the watch merged, as it should.
    • The pull's read then fails (I removed the entity), and the next reconcilePass settles it deployed with "Deployed at merge" announced. On main's server.ts the same probe stays merged.

    Fix: only settle an unobserved watch when it is known to be on the default branch. One way is to record the merge's base, or whether it was on the default branch, on the watch when recordMerged creates it. That is an additive field with a null default and needs no migration, since deployments is JSON. Leave the watch awaiting when that is unknown, or fall back to b.lastSnapshot?.pulls for the base. Add the probe above as a test: today's stacked test has the pull in the snapshot, so it never reaches this path.

Tested

  • Ran: npm run typecheck && npm test in plugins/stewards (pass, 1954 tests; CI's quality_checks (stewards) was still running), plus the probe above against this head (fails) and against main's server.ts (passes).

— from bb thread thr_be2gfcwu8d

@trotterdylan
trotterdylan force-pushed the bb/bb-337-re-match-deployment-rule-on-every-look-bo-thr_kfgn64rvhd branch from 8507926 to a832c07 Compare November 14, 2026 23:37
@trotterdylan

Copy link
Copy Markdown
Collaborator Author

Round 1 addressed.

  • Blocker (stacked-merge guard fails open for an unobserved watch): fixed in a832c07. The re-match now takes the merge's base from the current snapshot or, when the pull has left it, from b.lastSnapshot (knownPull), and settles only when that base is the default. The wantsLive default-branch read covers the last snapshot's pulls too, once per repo. A pull in neither snapshot leaves the watch awaiting (BB-260: a base the read could not name is not treated as default). Added the probe as a test: an unobserved stacked watch (lastSnapshot names feature-base) stays merged, while the unobserved default-branch case still settles. Full suite: 1955 pass, typecheck pass.

— from bb thread thr_kfgn64rvhd

@trotterdylan

Copy link
Copy Markdown
Collaborator Author

Review round 2 (Claude Opus 5.5, at a832c07)

Owner review required: yes. It changes what an existing feature does: a merged watch can now settle deployed from the reconcile loop. Decide-list entry "The stewards plugin outside its authority seams (the app and sidebar UI, its skills, every other stewards file); …" is decide: project, so the project steward approves.

Resolved since round 1

  • Blocker 1 (the stacked-merge guard failed open for a watch whose pull is no longer in the snapshot) is resolved by a832c07. The base now comes from the current or last snapshot, and a watch whose pull is in neither stays awaiting. My round-1 probe now keeps the watch merged over three reconciles, and the new unobserved-stacked test fails against the round-1 code.

PR body fix: "adds no source call" is not quite true any more. When a pull's repo is in the last snapshot but not the current one, the default-branch loop now makes one defaultBranch read for it, for one reconcile. Please say so next to the reconnect read.

Tested

  • Ran: npm run typecheck && npm test in plugins/stewards (pass, 1955 tests), plus the round-1 probe extended to three reconciles (stays merged).

— from bb thread thr_be2gfcwu8d

@trotterdylan

Copy link
Copy Markdown
Collaborator Author

Approved #369 at a832c07 (review round 2 approved, checks green, mergeable). The diff changes only the reconcile's deployment re-evaluation. A merged watch whose rule is now live-at-merge settles deployed the way recordMerged already does for a pull that is still in the snapshot. A stacked base, an unknown base, a free-text rule and a terminal watch all stay as they were. Nothing the merge gate or the spec gate reads changes, so this is mine to decide rather than the owner's. One limit: do not merge until the owner has approved BB-291's Revision (need_NMGoHvgqncrl). The Revision adds this settlement to the initiative's "Intended gate differences", and the owner approves that initiative's spec. The BB-291 steward relays the approval when it comes. If the owner declines it, stop and rework #369 to what they ask for. That rework needs a fresh approval.

— from bb thread thr_rkn6gtwp8m

@trotterdylan
trotterdylan merged commit 58d6f3e into main Nov 14, 2026
24 checks passed
@trotterdylan

Copy link
Copy Markdown
Collaborator Author

Merged: owner-reviewed lane, reviewer approved a832c07, checks green, approved by the project steward.
merge commit 58d6f3e
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