[trotterdylan]

Implements BB-108. With .bb a directory on the primary repo's default branch (BB-106), stewards' mergeLesson can no longer fast-forward a canonical .bb checkout, so a promoted or retired lesson now lands as a reviewed pull request on the primary repo.

Merging rebuilds the bb-server image and stages a restart for every developer's server. The restart happens at Restart now, or in the 03:00–05:00 ET quiet window. A .bb/stewards/project.json key that depends on this change waits for that restart.

Classification

owner-reviewed: the change touches plugins/stewards/server.ts and plugins/stewards/store.ts, which are in SELF_WIDENING_PATHS, so it is the user's to approve whatever bigDecisions says. It also changes existing behaviour (the Merge/Drop items become Review PR/Drop) and adds a store migration.

What changes

• A promoted or retired lesson becomes a pull request. The project steward still edits .bb/<file> and publishes the guidance-<thread> branch with workspace_publish_guidance; steward_update { lessonAction: complete | retire } then opens a pull request on the project source's primary repo (from repoSet.primary), against its default branch. The body names the lesson id, the anchor and the proposing steward, and is signed. Several lessons landed in one publish share one pull request.
• Who merges. A procedure lesson's PR is squash-merged by the project steward at once, no person asked. A boundary lesson's PR is filed for the owner as a Review PR / Drop item; the owner merges the PR, a project-steward approval never counts, and the lesson is marked merged when the PR merges (read through the github source, never a fast-forward). Drop closes the PR and marks the lesson dropped.
• Retire takes the same path: a retired anchor is a pull request (a fresh one, kept on retire_pr, not the promotion's).
• The self-widening guard holds, and the narrow merge guard is re-checked on the PR. A lesson may never target stewards/project.json, stewards/roles.json or bigDecisions; the PR path refuses before anything is opened, and the record-time guard refuses roles.json too. Before a procedure merge the plugin re-reads the PR: every changed path must be a file of a lesson published on that branch, and this lesson's file must carry the anchor the call names. Anything else is refused and noted, so a lesson can never smuggle a second change into an auto-merged PR.
• Stewards no longer calls mergeGuidance. The remaining fast-forward code stays in multi-repo for a standalone .bb repo but is unused here. wake.ts, the stewards skill, README.md and DESIGN.md describe the PR flow.
• Follow-up: the BB-107 native git project source (no multi-repo) is marked TODO(BB-107) in lessonRepo; until it lands, a project with no multi-repo reports the lesson PR refused rather than opening one. The standalone .bb layout's workspace_publish_guidance does not push the branch, so a PR there also reports the refusal.

Testing

• npm run typecheck in plugins/stewards: pass.
• npm test in plugins/stewards: pass (806 tests).
• New tests cover: a promoted procedure lesson opens exactly one PR on the primary repo carrying the anchored block and the project steward merges it; a refused createPull and a refused squashMerge leave the lesson pending with the reason; a PR that carries a second file or whose file lacks the anchor is refused before merging; a boundary lesson opens a PR, files Review PR/Drop and is never merged by the plugin, and a project-steward approval never stands in for the owner; the lesson is marked merged when its PR merges and dropped when Drop closes it; a retire opens a fresh PR and is observed when it merges or dropped via Drop; Drop on one of two lessons sharing a PR leaves it open; the project file and roles.json are refused before any PR; several lessons in one publish reuse one PR.
• Not tested: manual verification on the real repo. Which repo carries this project's .bb is still the user's open question (BB-106), so no lesson PR is opened on the public bitcomplete/bb-plugins; the tests use a fixture repo (the injected GithubWriter).

— from bb thread thr_y6zg5g7b43

[trotterdylan]

Independent review running in bb thread @thread:thr_98b6i7pqjx.

— from bb thread thr_y6zg5g7b43

[trotterdylan]

Review round 1 (GLM 5.3, at 5bc55a4)

Owner review required: yes — the change touches plugins/stewards/server.ts and plugins/stewards/store.ts, which are hard-coded in SELF_WIDENING_PATHS (decisions.ts), so it is the user's to approve whatever the decide list says; it also changes existing behaviour (lesson landing, the Merge/Drop items) and adds a store migration. @trotterdylan, this one needs your approval.

Blockers

1. server.ts:3827–3832 (with server.ts:5692 and server.ts:3870) — a boundary lesson's retirement strands the pull request, the row and the owner's item. retireLesson for a boundary lesson opens a fresh PR and files a Review PR/Drop need — but retireLesson requires lesson.status === "merged" (line 3821), and the need handler checks lesson.status !== "pending" before the Drop branch (line 5692). So the owner's Drop on that item returns "That lesson is already resolved." and closes the need without ever reaching closeLessonPr — the retire PR is orphaned open. Worse, reconcileLessonPrs (line 3870) skips non-pending lessons, so when the owner merges the retire PR on GitHub nothing marks the lesson dropped or closes the item. The row stays status: "merged", which is exactly the ## Lessons to retire filter (tier === "retired" && status === "merged" in wake.ts), so the project steward is re-told to retire it every night, and every retry opens or refuses a PR against an already-merged branch. The old code marked the row dropped for both scopes; this is a regression on a designed path (DESIGN.md: "a boundary one waits for the owner") with no test covering a boundary retire — every retire test uses the promoted() helper, which is procedure. Fix: make reconcileLessonPrs also observe a retired-tier lesson's PR and mark it dropped/close the need when it merges, and let Drop on a retire item close the PR and drop the row (or don't file a Review PR/Drop item for a retire at all if the owner's merge is observed directly). Add a test for the boundary retire, PR merged and Drop both.

2. server.ts:3776–3782 (closeLessonPr) — Drop on one lesson closes the pull request other pending lessons still need. Several lessons in one publish deliberately share one PR (tested in "reuses one pull request when several lessons are landed in the same publish"), and Drop is a per-lesson choice on each of their items. Dropping one calls closeLessonPr, which closes the shared PR unconditionally; the surviving pending sibling keeps pr set, so openLessonPr (line 3737) returns "already open at " forever — the plugin never reopens it or opens a fresh one, and the owner is pointed at a closed PR. The sibling's landing path is destroyed by an unrelated Drop and can only be recovered by hand. Fix: only close the PR when no other pending lesson references the same URL (or, if shared, leave it open and let the owner's merge of it land the rest), and add a test for Drop on one of the shared lessons.

Suggestions

• server.ts:3768 — mergeLessonPr does not catch a squashMerge failure (e.g. the PR is not mergeable or a branch protection rule refuses). The error escapes the tool call and noteLesson is never run, so the nightly wake cannot show "Last attempt" the way a refused createPull does. Wrap it and record the note as a refusal.
• server.ts:3690–3702 — lessonRepo reads layout from the repo set but never uses it, and both the standalone-layout and the non-GitHub-primary.url cases report "could not read the project's primary repo; is multi-repo installed?", which is misleading when multi-repo is installed but the project is standalone. Distinguish the messages or drop the unused read.

Checked

• Ran: npm run typecheck and npm test in plugins/stewards (800/800 pass, ~73s). stewards is in the plugins workflow matrix, so CI covers it; no other plugin's files change, so nothing was skipped.
• Verified against ticket:
  • Promoted lesson opens exactly one PR on the primary repo with the anchored block — met (test asserts repo, head, base, body anchor, signature; exactly one).
  • Project file / roles.json (and so bigDecisions, which lives in the project file) refused before any push — met (record-time refine and the openLessonPr guard, both tested with pre-guard rows).
  • Marked merged when the PR merges, dropped when Drop closes the PR — met for the single-lesson case; blocked by blockers 1 and 2 for the boundary-retire and shared-PR cases.
  • A retire opens a PR — met for procedure; blocked by blocker 1 for boundary.
  • mergeGuidance has no remaining call site in stewards — met (grep: only multi-repo defines it).
  • Manual verification on the real repo — correctly skipped and stated in the PR body, per the BB-106 open question; the fixture GithubWriter never touches the public repo.
• Also confirmed multi-repo's repoSet already returns primary and layout (BB-106, contract.ts:614–617), so the RPC read is sound; the store migration is append-only and additive (pr column, defaulted, previous build compatible); and the Lesson_pr migration-order tests were updated coherently.

— from bb thread thr_98b6i7pqjx

[trotterdylan]

Round 2 fixes at 6f629b3:

Blocker 1 — boundary retire stranded its PR, row and item. Fixed. A retire PR is now kept on a new retire_pr column (LESSON_PR_MIGRATIONS), separate from the promotion pr, so a promotion merge never retires and a retire merge never lands. reconcileLessonPrs observes a retired lesson's retirePr and marks the row dropped and closes the item when it merges; it also runs even when there is no pending lesson (the old reconcileLessons early return skipped it). The Review PR/Drop item for a retire carries data.retire and is accepted by the need handler: Drop closes the retire PR and drops the row, Review PR points at it. Tests added: boundary retire opens a fresh PR and is observed when it merges; Drop on a boundary retire closes the PR and drops the row.

Blocker 2 — Drop closed a shared PR. Fixed. closeLessonPr now closes a PR only when no other pending lesson references the same URL; a shared promotion PR is left open for the survivors. Test added: Drop on one of two lessons sharing a PR leaves it open and the sibling pending.

Suggestion — mergeLessonPr swallowed a failed squash-merge. Fixed: it catches, records the note on the lesson, and returns pull request refused: …, so the nightly wake shows "Last attempt". Test added.

Suggestion — lessonRepo unused layout and misleading message. Fixed: lessonRepo no longer reads layout and returns distinct errors for no origin, a non-GitHub origin, and an unreadable repo set.

npm run typecheck and npm test in plugins/stewards pass (804 tests).

— from bb thread thr_y6zg5g7b43

[trotterdylan]

Review round 2 (GLM 5.3, at 6f629b3)

Blockers

1. server.ts:3763 and 3774 (openLessonPr/mergeLessonPr) — the plugin squash-merges a procedure lesson's pull request without ever checking what it changes, reopening the "smuggle in a second change" hole the old path refused. openLessonPr never reads the PR's changed files (github.files/pullChangedFiles is used only for ticket PRs, lines 896–901), and mergeLessonPr squash-merges unconditionally once createPull succeeded. The old mergeGuidance (multi-repo source.ts:685–690, still present) refused "a commit range that changes anything but file, or a file that does not carry text" — "a self-merged lesson may therefore only ever add its own wording to its own file; it cannot smuggle in a second change." The new DESIGN.md:1546 claims the property is kept: "The pull request changes only the lesson's own file and carries the anchor the call names, and a reviewer sees the diff like any other change" — but nothing enforces the first half, and the second half is false for a procedure lesson, which the plugin merges in the same tool call with no person asked. So whatever else the published guidance-<thread> branch carries — an unrelated playbook edit, or an edit to .bb/stewards/project.json or stewards/roles.json — lands on the primary repo's default branch without a human seeing it. That is exactly the BB-35 self-widening hole: a lesson cannot target the project file, but its PR can carry an edit to it and be auto-merged. Fix: after createPull (and before squashMerge), read the PR's changed files with github.files and refuse unless every changed path is one of the files of the lessons the PR lands (the lesson's own file, plus sibling lessons sharing the branch/PR — they may be in different files). If the "carries the anchor" half is not also enforced, soften the DESIGN line rather than leaving the claim unenforced. Add a test whose PR reports a smuggled second file and expects the refusal, with the lesson left pending and noted.

Suggestions

• server.ts:3838 (retireLesson) — no "already retired at .bb HEAD" observation, unlike completeLesson's atHead check. When two lessons are retired in one publish (or a procedure retire shares a branch with a boundary retire), the first PR's merge carries both retire edits. The second lesson then has status: merged, tier: retired, retirePr: null; its retry createPull is refused ("no commits between…"), it is re-listed under ## Lessons to retire every night, and nothing observes that the retired anchor is already at HEAD — the row only settles if the steward drops it by hand. Mirroring the atHead check (mark dropped when the file at .bb HEAD already carries the retired anchor) would close it.

Resolved since round 1

• Blocker 1 (boundary retire strands the PR, row and item): resolved — retirePr is kept apart from pr, reconcileLessonPrs observes a merged-status lesson's retire PR and drops the row/closes the item when it merges, and the need/CLI handlers accept a retire item (isRetire), with Drop closing the retire PR and dropping the row. Tests cover both the observed merge and Drop.
• Blocker 2 (Drop closes a PR shared by other pending lessons): resolved — closeLessonPr leaves the PR open when another pending lesson references it; test drops one of two shared lessons and confirms the survivor keeps the open PR.
• Suggestion (unhandled squashMerge failure): resolved — refused merges are recorded as a lesson note, with a test.
• Suggestion (lessonRepo's unused layout and misleading message): resolved — distinct messages for the standalone-layout and non-GitHub-origin cases.

Checked

• Ran: npm run typecheck and npm test in plugins/stewards (804/804 pass; stewards is in the CI matrix, no skipped suites).
• Re-read the full 5bc55a4..6f629b3 diff: both fix commits stay within the reviewed scope (the retirePr store column is additive and defaulted, and the migration-order tests are untouched — LESSON_PR_MIGRATIONS still last); the new isRetire guards in the page and CLI paths are consistent; reconcileLessons now reaches reconcileLessonPrs even with no pending lessons.
• Verified against ticket: all criteria still met (exactly-one PR, refused targets, merged-on-merge/dropped-on-Drop, retire opens a PR, no mergeGuidance call site, manual verification skipped as agreed); the new blocker is about what the PR may contain, not whether it opens.

— from bb thread thr_98b6i7pqjx

[trotterdylan]

Round 3 fixes at c64b664:

Blocker — the procedure merge did not check what the PR changes. Fixed. mergeLessonPr now runs verifyLessonPr before squashMerge: it reads the PR's changed files and refuses unless every path is a file of a lesson published on that branch (so siblings in one publish are allowed), and then reads this lesson's file at the branch head and refuses unless it carries the anchor the call names. The check fails closed when github.files/github.fileContent are unavailable, and refuses a full first page of files (a possible second page). A refusal is noted on the lesson like a refused open. Tests added: a PR reporting .bb/stewards/project.json alongside .bb/AGENTS.md is refused and not merged; a PR whose file lacks the anchor is refused and not merged. DESIGN.md's guard bullet now states the file-list and anchor checks the merge enforces.

Suggestion — retire had no "already retired at .bb HEAD" observation. Fixed: retireLesson now marks the row dropped and closes the item when the file at .bb HEAD already carries the retired anchor, mirroring completeLesson.

npm run typecheck and npm test in plugins/stewards pass (806 tests).

— from bb thread thr_y6zg5g7b43

[trotterdylan]

Review round 3 (GLM 5.3, at c64b664)

Owner review required: yes — the change touches plugins/stewards/server.ts and plugins/stewards/store.ts (SELF_WIDENING_PATHS in decisions.ts), so it is the user's to approve whatever the decide list says; it also changes existing lesson-landing behaviour and adds store columns. @trotterdylan, this one needs your approval before it merges.

Suggestions

• server.ts:3771 (lessonPrFiles) — the allowed set is branch-wide, wider than the old guard. branch is recorded on every lesson at record time, so the allowed files are effectively "any file any lesson in this project ever used". A smuggled edit to a playbook that some past lesson targeted would still pass the guard unreviewed, where the old fast-forward refused any second file. Tightening to the lessons that actually share this PR (pr/retirePr equal to result.url) would restore the old precision, and filtering lessonFileAllowed(l.file) would keep a hand-edited pre-guard row (file = stewards/project.json) from ever widening the set. Non-blocking: the project file and roles.json are refused through every normal path, and DESIGN.md now documents the implemented rule honestly.

Resolved since round 2

• Blocker 1 (plugin squash-merged a procedure lesson's PR without checking its diff): resolved — verifyLessonPr runs before every plugin merge (procedure complete and procedure retire), fails closed when the PR, its files or the file content cannot be read, refuses a full page of changed files, refuses any changed path outside the lesson set (.bb/-prefixed or not), and refuses a file that does not carry the anchor; the refusal is noted on the lesson. Tests cover a smuggled .bb/stewards/project.json and a missing anchor. DESIGN.md's guard paragraph now states the enforced rule.
• Suggestion (no "already retired at .bb HEAD" observation): resolved — retireLesson now drops the row and closes the item when the retired anchor is already at .bb HEAD, mirroring completeLesson's atHead check.

Checked

• Ran: npm run typecheck and npm test in plugins/stewards (806/806 pass; stewards is in the CI matrix, nothing skipped).
• Verified against ticket:
  • A promoted lesson opens exactly one PR on the primary repo containing the anchored block — met, and the plugin now verifies the anchor before its own merge.
  • A lesson targeting the project file / roles.json / bigDecisions is refused before any push — met (record-time, PR-open-time, and now merge-time: a smuggled project.json in the PR is refused too).
  • Marked merged when its PR merges, dropped when Drop closes the PR — met, including the boundary retire (observed merge and Drop both tested) and the shared-PR case (Drop leaves it open for the sibling).
  • A retire opens a PR — met, with the at-HEAD shortcut for an already-removed anchor.
  • mergeGuidance has no remaining call site in stewards — met.
  • Manual verification on the real repo — skipped and stated in the PR body, as agreed for BB-106.
• No blockers remain. The one suggestion above may be declined with a reason.

— from bb thread thr_98b6i7pqjx

[trotterdylan]

LGTM

[trotterdylan]

Review round 4 (GLM 5.3, at eb2a898)

Owner review required: yes — plugins/stewards/server.ts and store.ts are in SELF_WIDENING_PATHS, so @trotterdylan's approval is the gate, as named in round 3.

This push only implements my round-3 suggestion: lessonPrFiles now allows exactly the current lesson's file plus the files of lessons sharing this PR (pr/retirePr equal to it), filtered by lessonFileAllowed so a pre-guard project.json row cannot widen the set. No scope or behaviour beyond what was reviewed; the round-3 approval stands, repeated on this head.

Checked

• Ran: npm run typecheck and npm test in plugins/stewards (806/806 pass).
• No blockers; no open suggestions.

— from bb thread thr_98b6i7pqjx

[trotterdylan]

Merged: owner-reviewed, approved by trotterdylan on eb2a898 (unsigned LGTM comment 2026-10-28T19:34:08Z), reviewer approved eb2a898 (round 4), checks green. merge commit f8fd1de.

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

— from bb thread thr_y6zg5g7b43