From b4a868106e98f252b33ab23685f7e1c5b6004cac Mon Sep 17 00:00:00 2001 From: Dylan Trotter Date: Tue, 10 Nov 2026 12:00:09 +0000 Subject: [PATCH 1/4] stewards: lessons land by the decide list, guidance units carry gu_ ids (BB-288) Renders the `gu_` id beside every AGENTS.md and playbook guidance unit a wake or brief shows, so a steward can cite the guidance it followed; lists trial lessons the project steward may land early, with the command, and keeps them out of the owner's digest priorities; places a lesson's pull request by the ordinary decide list (a playbook is the project steward's, AGENTS.md is the owner's) with the ordinary project approval recorded; and suppresses the efficacy "uncited for 90 days" list until `gu_` ids have rendered that long, from a stored first-render date. --- plugins/stewards/lessons.test.ts | 33 +++++- plugins/stewards/lessons.ts | 26 ++++- plugins/stewards/server.test.ts | 69 ++++++++++-- plugins/stewards/server.ts | 122 ++++++++++++++++++---- plugins/stewards/skills/stewards/SKILL.md | 8 +- plugins/stewards/wake.test.ts | 34 ++++++ plugins/stewards/wake.ts | 41 +++++++- 7 files changed, 295 insertions(+), 38 deletions(-) diff --git a/plugins/stewards/lessons.test.ts b/plugins/stewards/lessons.test.ts index 8f71821e..a8eb5099 100644 --- a/plugins/stewards/lessons.test.ts +++ b/plugins/stewards/lessons.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "vitest"; -import { hasLessonAnchor, lessonAnchor, lessonScore, planLessonTransitions, planValidations, removeLessonAnchor, retireLessonAnchor, retiredLessonAnchor, withLessonAnchor, type ScoreableLesson } from "./lessons.js"; +import { hasLessonAnchor, landLessonInstruction, lessonAnchor, lessonLandingMark, lessonScore, planLessonTransitions, planValidations, removeLessonAnchor, retireLessonAnchor, retiredLessonAnchor, withLessonAnchor, type ScoreableLesson } from "./lessons.js"; import type { LessonRules } from "./config.js"; const DAY = 86_400_000; @@ -137,3 +137,34 @@ describe("the validation queue (BB-35 step 5)", () => { expect(planValidations([weak, strong], rules, T).map((r) => r.id)).toEqual(["lsn_strong", "lsn_weak"]); }); }); + +describe("the landing decision (BB-288)", () => { + // The decide-list entries bb-gate `main` carries (BB-280): the `user` entry + // for AGENTS.md and the `project` entry for the playbooks. A lesson is placed + // by these, not by its scope, except that a boundary lesson stays the user's. + const decisions = [ + { text: "Who stewards are and what the project tells them (steward roles, the decide list, the project's guidance)", decide: "user" as const, paths: [".bb/stewards/roles.json", ".bb/stewards/project.json", ".bb/AGENTS.md"] }, + { text: "What every steward is told (the playbooks and the project's skills)", decide: "project" as const, paths: [".bb/stewards/playbooks/**", ".bb/skills/**"] }, + ]; + + it("places a procedure lesson by the ordinary decide list", () => { + expect(lessonLandingMark({ file: "stewards/playbooks/linear-tickets.md", scope: "procedure" }, decisions)).toBe("project"); + expect(lessonLandingMark({ file: "AGENTS.md", scope: "procedure" }, decisions)).toBe("user"); + }); + + it("keeps a boundary lesson the user's whatever file it targets", () => { + expect(lessonLandingMark({ file: "stewards/playbooks/linear-tickets.md", scope: "boundary" }, decisions)).toBe("user"); + }); + + it("fails closed on no match or an unreadable decide list", () => { + expect(lessonLandingMark({ file: "stewards/playbooks/linear-tickets.md", scope: "procedure" }, [])).toBe("user"); + expect(lessonLandingMark({ file: "docs/whatever.md", scope: "procedure" }, decisions)).toBe("user"); + }); + + it("names the anchor, file and completion call for an early landing", () => { + const instruction = landLessonInstruction({ id: "lsn_1", file: "stewards/playbooks/linear-tickets.md" }); + expect(instruction).toContain(""); + expect(instruction).toContain("stewards/playbooks/linear-tickets.md"); + expect(instruction).toContain('steward_update { lessonAction: { id: "lsn_1", action: "complete" } }'); + }); +}); diff --git a/plugins/stewards/lessons.ts b/plugins/stewards/lessons.ts index 65c40c06..3dc7a3bc 100644 --- a/plugins/stewards/lessons.ts +++ b/plugins/stewards/lessons.ts @@ -3,8 +3,9 @@ // lessons, and the thresholds and caps that decide which tier a lesson moves // to. Nothing here touches the store or a plugin; server.ts applies what this // returns. Keeping it pure is what makes every transition a unit test. -import type { LessonScope, LessonTier } from "./store.js"; +import type { Lesson, LessonScope, LessonTier } from "./store.js"; import type { LessonRules } from "./config.js"; +import { decideForPull, type BigDecision, type DecideMark } from "./decisions.js"; const DAY_MS = 86_400_000; @@ -166,6 +167,29 @@ export function planLessonTransitions(lessons: readonly ScoreableLesson[], rules return out; } +/** + * How a lesson's pull request is placed (BB-288): the ordinary decide list + * applied to the lesson's own `.bb` file, never a lesson-specific rule. A + * `boundary` lesson is always the user's, because it changes the escalation + * boundary. A `procedure` lesson in a file the decide list marks `project` (a + * playbook) is the project steward's; one in a `user` file (`AGENTS.md`, the + * project file, `roles.json`) is the owner's. The files are fail-closed: an + * unreadable list or no match is the user's, exactly as `decideForPull`. + */ +export function lessonLandingMark(lesson: Pick, decisions: readonly BigDecision[]): DecideMark { + if (lesson.scope === "boundary") return "user"; + return decideForPull([lesson.file], decisions); +} + +/** + * The sentence a wake and the record-time reply use to land a trial lesson + * early, before the nightly pass promotes it (BB-288). It names the anchor, the + * file and the completion call, so both surfaces say the same thing. + */ +export function landLessonInstruction(lesson: Pick): string { + return `To land it early: add \`${lessonAnchor(lesson.id)}\` and the wording to \`${lesson.file}\`, publish with \`workspace_publish_guidance\` (on a native project, commit and open the pull request yourself), then complete it with \`steward_update { lessonAction: { id: "${lesson.id}", action: "complete" } }\`.`; +} + /** The two rare conditions a validation escalation asks the user about (BB-35 step 5). */ export const VALIDATION_KINDS = ["boundary-promotion", "mixed-evidence"] as const; export type ValidationKind = (typeof VALIDATION_KINDS)[number]; diff --git a/plugins/stewards/server.test.ts b/plugins/stewards/server.test.ts index 94feb65b..0196443f 100644 --- a/plugins/stewards/server.test.ts +++ b/plugins/stewards/server.test.ts @@ -14,6 +14,24 @@ import { MIGRATIONS, ESCALATED_NEED_KINDS, NEEDS_KINDS, STEWARD_NEED_KINDS, type import { GITHUB_ISSUES, GITHUB_PRS, GITHUB_REVIEWS, LINEAR_TICKETS, OPS_WATCH, DEPS_ALERTS, DEPS_PRS, PLAYBOOK_TEMPLATES, projectFileTemplate } from "./templates.js"; import { STANDING_RULES } from "./wake.js"; +/** + * The decide-list entry BB-280 landed on bb-gate `main` for playbooks (read + * from `.bb/stewards/project.json`), so a lesson in a playbook is the project + * steward's to land (BB-288). The end-to-end test uses the entry that is + * actually there, not an assumed one. + */ +const BB280_PLAYBOOK_ENTRY = { text: "What every steward is told (the playbooks and the project's skills)", decide: "project", paths: [".bb/stewards/playbooks/**", ".bb/skills/**"] } as const; +/** The `user` entry the same real project file carries for `AGENTS.md`, so the test proves the `user` entry wins (BB-288). */ +const AGENTS_USER_ENTRY = { text: "Who stewards are and what the project tells them (steward roles, the decide list, the project's guidance)", decide: "user", paths: [".bb/stewards/roles.json", ".bb/stewards/project.json", ".bb/AGENTS.md"] } as const; +/** A `.bb`-relative playbook file a lesson can target: the `project` entry's path (BB-288). */ +const LESSON_PLAYBOOK_FILE = playbookPath("linear-tickets"); +/** Add the real decide-list entries a lesson is placed by to a world's project file (BB-288). */ +function withBb280PlaybookEntry(t: { files: Record }): void { + const parsed = JSON.parse(t.files[PROJECT_FILE] ?? "{}") as { bigDecisions?: unknown[] }; + parsed.bigDecisions = [...(parsed.bigDecisions ?? []), AGENTS_USER_ENTRY, BB280_PLAYBOOK_ENTRY]; + t.files[PROJECT_FILE] = JSON.stringify(parsed); +} + type Call = { pluginId: string; method: string; input: Record }; // A world of fake neighbours: multi-repo serving `.bb`, linear and github @@ -10539,6 +10557,7 @@ describe("GitHub filing targets (BB-235)", () => { describe("lessons", () => { async function adopted() { const t = await setup(); + withBb280PlaybookEntry(t); t.entities.set("uuid-12", issue()); await t.ring("linear", "uuid-12"); const [project, steward] = t.spawns.map((x) => x.id) as [string, string]; @@ -10582,7 +10601,7 @@ describe("lessons", () => { it("a procedure lesson opens one pull request on the primary repo and the project steward merges it", async () => { const t = await adopted(); - const recorded = await t.tool("steward_update", { binding: "ENG-12", lesson: { text: lesson, file: "AGENTS.md", scope: "procedure" } }, t.project); + const recorded = await t.tool("steward_update", { binding: "ENG-12", lesson: { text: lesson, file: LESSON_PLAYBOOK_FILE, scope: "procedure" } }, t.project); const id = /(lsn_[\w-]+)/u.exec(recorded.text)![1]!; const done = await t.tool("steward_update", { lessonAction: { id, action: "complete" } }, t.project); expect(done).toMatchObject({ error: false, text: expect.stringMatching(new RegExp(`${id}: merged at c{8}`, "u")) }); @@ -10598,10 +10617,25 @@ describe("lessons", () => { expect(t.hooks.store.lesson(id)).toMatchObject({ status: "merged", tier: "promoted", mergeSha: "c".repeat(40), branch: `refs/heads/guidance-${t.project}`, pr: pr.url }); }); + it("a procedure lesson in a user-decided file is the owner's, not the project steward's (BB-288)", async () => { + const t = await adopted(); + const recorded = await t.tool("steward_update", { binding: "ENG-12", lesson: { text: lesson, file: "AGENTS.md", scope: "procedure" } }, t.project); + const id = /(lsn_[\w-]+)/u.exec(recorded.text)![1]!; + const done = await t.tool("steward_update", { lessonAction: { id, action: "complete" } }, t.project); + expect(done.text).toMatch(/decide list makes this the user's/u); + // The pull request is open, but the project steward never merges it: the + // owner's Review PR/Drop item is filed instead. + expect(t.githubPulls).toHaveLength(1); + expect(t.githubMerges).toEqual([]); + expect(t.hooks.store.lesson(id)).toMatchObject({ status: "pending" }); + const need = t.hooks.store.openNeeds(t.hooks.store.find("ENG-12")!.id).find((n) => n.kind === "decide" && n.data.lessonId === id)!; + expect(need).toMatchObject({ data: { choices: ["Review PR", "Drop"] } }); + }); + it("a refused pull request leaves the lesson pending with the reason", async () => { const t = await adopted(); t.setCreatePullError("No commits between main and guidance-x."); - const recorded = await t.tool("steward_update", { binding: "ENG-12", lesson: { text: lesson, file: "AGENTS.md", scope: "procedure" } }, t.project); + const recorded = await t.tool("steward_update", { binding: "ENG-12", lesson: { text: lesson, file: LESSON_PLAYBOOK_FILE, scope: "procedure" } }, t.project); const id = /(lsn_[\w-]+)/u.exec(recorded.text)![1]!; const done = await t.tool("steward_update", { lessonAction: { id, action: "complete" } }, t.project); expect(done.text).toMatch(/pull request refused: No commits between main and guidance-x\./u); @@ -10611,7 +10645,7 @@ describe("lessons", () => { it("a refused merge leaves the lesson pending with the reason", async () => { const t = await adopted(); t.setMergePullError("Pull request is not mergeable: required status check is expected."); - const recorded = await t.tool("steward_update", { binding: "ENG-12", lesson: { text: lesson, file: "AGENTS.md", scope: "procedure" } }, t.project); + const recorded = await t.tool("steward_update", { binding: "ENG-12", lesson: { text: lesson, file: LESSON_PLAYBOOK_FILE, scope: "procedure" } }, t.project); const id = /(lsn_[\w-]+)/u.exec(recorded.text)![1]!; const done = await t.tool("steward_update", { lessonAction: { id, action: "complete" } }, t.project); expect(done.text).toMatch(/pull request refused: Pull request is not mergeable/u); @@ -10620,10 +10654,10 @@ describe("lessons", () => { it("refuses to merge a procedure lesson's pull request that carries a second file", async () => { const t = await adopted(); - const recorded = await t.tool("steward_update", { binding: "ENG-12", lesson: { text: lesson, file: "AGENTS.md", scope: "procedure" } }, t.project); + const recorded = await t.tool("steward_update", { binding: "ENG-12", lesson: { text: lesson, file: LESSON_PLAYBOOK_FILE, scope: "procedure" } }, t.project); const id = /(lsn_[\w-]+)/u.exec(recorded.text)![1]!; // The published branch smuggled an edit to the project file alongside the lesson. - t.githubFiles.set("acme/bb#1", [".bb/AGENTS.md", ".bb/stewards/project.json"]); + t.githubFiles.set("acme/bb#1", [`.bb/${LESSON_PLAYBOOK_FILE}`, ".bb/stewards/project.json"]); const done = await t.tool("steward_update", { lessonAction: { id, action: "complete" } }, t.project); expect(done.text).toMatch(/pull request refused: .*other than the lesson's own/u); expect(t.githubMerges).toEqual([]); @@ -10632,9 +10666,9 @@ describe("lessons", () => { it("refuses to merge a procedure lesson's pull request whose file lacks the anchor", async () => { const t = await adopted(); - const recorded = await t.tool("steward_update", { binding: "ENG-12", lesson: { text: lesson, file: "AGENTS.md", scope: "procedure" } }, t.project); + const recorded = await t.tool("steward_update", { binding: "ENG-12", lesson: { text: lesson, file: LESSON_PLAYBOOK_FILE, scope: "procedure" } }, t.project); const id = /(lsn_[\w-]+)/u.exec(recorded.text)![1]!; - t.githubFileContents.set(`acme/bb#guidance-${t.project}:.bb/AGENTS.md`, "# Guidance\n\nNo anchor here.\n"); + t.githubFileContents.set(`acme/bb#guidance-${t.project}:.bb/${LESSON_PLAYBOOK_FILE}`, "# Guidance\n\nNo anchor here.\n"); const done = await t.tool("steward_update", { lessonAction: { id, action: "complete" } }, t.project); expect(done.text).toMatch(/pull request refused: .*does not carry the lesson's anchor/u); expect(t.githubMerges).toEqual([]); @@ -10857,7 +10891,7 @@ describe("lessons", () => { it("promotes a trial lesson the nightly pass finds ready, and lands its anchored block", async () => { const t = await adopted(); - const recorded = await t.tool("steward_update", { binding: "ENG-12", lesson: { text: lesson, file: "AGENTS.md", scope: "procedure" } }, t.project); + const recorded = await t.tool("steward_update", { binding: "ENG-12", lesson: { text: lesson, file: LESSON_PLAYBOOK_FILE, scope: "procedure" } }, t.project); const id = /(lsn_[\w-]+)/u.exec(recorded.text)![1]!; // Three distinct bindings cite it, which is the promotion's hit threshold. for (const key of ["ENG-13", "ENG-14", "ENG-15"]) { @@ -10877,7 +10911,7 @@ describe("lessons", () => { it("retires an aged promoted lesson, and removes its anchored text through the same path", async () => { const t = await adopted(); - const recorded = await t.tool("steward_update", { binding: "ENG-12", lesson: { text: lesson, file: "AGENTS.md", scope: "procedure" } }, t.project); + const recorded = await t.tool("steward_update", { binding: "ENG-12", lesson: { text: lesson, file: LESSON_PLAYBOOK_FILE, scope: "procedure" } }, t.project); const id = /(lsn_[\w-]+)/u.exec(recorded.text)![1]!; await t.tool("steward_update", { lessonAction: { id, action: "complete" } }, t.project); expect(t.hooks.store.lesson(id)).toMatchObject({ tier: "promoted", status: "merged" }); @@ -10963,6 +10997,7 @@ describe("validation escalations (BB-35 step 5)", () => { const text = "Only the user may grant a credential."; async function adopted() { const t = await setup(); + withBb280PlaybookEntry(t); t.entities.set("uuid-12", issue()); await t.ring("linear", "uuid-12"); const [project, steward] = t.spawns.map((x) => x.id) as [string, string]; @@ -10977,7 +11012,7 @@ describe("validation escalations (BB-35 step 5)", () => { } /** A procedure lesson landed in `.bb`, promoted, then given mixed evidence. */ async function promoted(t: Awaited>, hits = 4, corrections = 1) { - const recorded = await t.tool("steward_update", { binding: "ENG-12", lesson: { text, file: "AGENTS.md", scope: "procedure" } }, t.project); + const recorded = await t.tool("steward_update", { binding: "ENG-12", lesson: { text, file: LESSON_PLAYBOOK_FILE, scope: "procedure" } }, t.project); const id = /(lsn_[\w-]+)/u.exec(recorded.text)![1]!; await t.tool("steward_update", { lessonAction: { id, action: "complete" } }, t.project); for (let i = 0; i < hits; i++) t.hooks.store.recordCitations("p1", [id], `bnd_${i}`); @@ -11285,6 +11320,9 @@ describe("guidance units and the efficacy view (BB-35 step 6)", () => { const t = await adopted(); const text = "A new column read with SELECT * breaks the previous image."; t.files["AGENTS.md"] = `# Guidance\n\n${text}\n`; + // The guidance index is cached with the config's TTL; the wake at adoption + // read the old file, so expire it before the citation re-reads (BB-288). + t.advance(6 * 60_000); const unitId = guidanceUnitId(text); await t.tool("steward_report", { phase: "Todo", summary: "Scope unclear.", nextAction: "Wait", actor: "other", escalate: { kind: "decision", question: "Include SSO?", meanwhile: "Wait." } }, t.steward); const esc = /(esc_[\w-]+) from ENG-12/u.exec(t.sends.filter((s) => s.threadId === t.project).at(-1)!.text)![1]!; @@ -11299,6 +11337,13 @@ describe("guidance units and the efficacy view (BB-35 step 6)", () => { const cited = "Cited recently."; const uncited = "Nothing cites this yet."; t.files["AGENTS.md"] = `# Guidance\n\n${cited}\n\n${uncited}\n`; + // BB-288: a fresh world has just rendered `gu_` ids, so the "uncited for 90 + // days" list is suppressed and says so rather than naming every unit. + const suppressed = await t.harness.behavior.runCli(["lessons", "--efficacy", "--project", "p1"]); + expect(suppressed.stdout).toMatch(/suppressed/u); + expect(suppressed.stdout).not.toContain(guidanceUnitId(uncited)); + // After the ids have rendered for 90 days, the list appears. + t.advance(91 * 86_400_000); // One forwarded decision (its escalation and the page's question are one // item), and one unit cited so it is not uncited. await forward(t); @@ -13877,6 +13922,8 @@ describe("the reviewer role and the gate and merge commands (BB-133)", () => { const withNote = await withPull([reviewMarker("no-review")], { repo: "bitcomplete/bb-plugins", noReconcileGate: true }); // The real `.bb/AGENTS.md` shape ("Who merges"): all three in one paragraph. withNote.files["AGENTS.md"] = "The lane decides who approves, not what ships. **Any merge to `bb-plugins` `main`, docs included, moves the pin**, builds a new `bb-server` image and stages a restart for every developer.\n"; + // The wake at setup cached the old guidance; expire it before the merge reads (BB-288). + withNote.advance(6 * 60_000); const merged = await withNote.harness.behavior.runCli(["merge", "bitcomplete/bb-plugins#7", "--project", "p1"]); expect(merged.exitCode).toBe(0); expect(withNote.githubComments.at(-1)!.body).toMatch(/moves the pin and stages a restart/u); @@ -13885,6 +13932,7 @@ describe("the reviewer role and the gate and merge commands (BB-133)", () => { // every-merge claim, so no note: the false positive the reviewer found. const devbox = await withPull([reviewMarker("no-review")], { repo: "bitcomplete/devbox", noReconcileGate: true }); devbox.files["AGENTS.md"] = "## Change classification\n\nA change to `devbox/host/**` is production infrastructure. A merge rebuilds the `bb-server` image and restarts every developer's server.\n"; + devbox.advance(6 * 60_000); const devboxMerged = await devbox.harness.behavior.runCli(["merge", "bitcomplete/devbox#7", "--project", "p1"]); expect(devboxMerged.exitCode).toBe(0); expect(devbox.githubComments.at(-1)!.body).not.toMatch(/moves the pin/u); @@ -13893,6 +13941,7 @@ describe("the reviewer role and the gate and merge commands (BB-133)", () => { // section, is not the project saying a merge to that repo restarts servers. const split = await withPull([reviewMarker("no-review")], { repo: "bitcomplete/bb-gate", noReconcileGate: true }); split.files["AGENTS.md"] = "## Shipping a plugin change\n\nLanding on `bb-gate` main is not instant; the pin is rewritten.\n\nEvery developer's server will restart when a staged image lands.\n"; + split.advance(6 * 60_000); const splitMerged = await split.harness.behavior.runCli(["merge", "bitcomplete/bb-gate#7", "--project", "p1"]); expect(splitMerged.exitCode).toBe(0); expect(split.githubComments.at(-1)!.body).not.toMatch(/moves the pin/u); diff --git a/plugins/stewards/server.ts b/plugins/stewards/server.ts index c9b52f56..f9367ea8 100644 --- a/plugins/stewards/server.ts +++ b/plugins/stewards/server.ts @@ -97,7 +97,7 @@ import { classify, hasFingerprintLine, lookChanges, opsTargetKey, parseOpsTarget import { EVENTS_PER_LOOK, NO_TOKEN, RECENT_PER_LOOK, SENTRY_API_URL, TOKEN_SCOPES, createSentryReader, emptySentryLook, issueInOrg, recentLine, sentryChanges, sentryResolveTargets, sentrySightings, sentrySummary, type SentryFinding, type SentryLook, type SentryReader } from "./sentry.js"; import { advanced, answerDiffers, anyWorkerInFlight, asksForOpenApproval, commentDiffersFromRecommendation, compactDue, deploymentSettled, diffObserved, endingDeadline, followingThrough, gateKnownUnmet, hasLabel, IDLE_WAKE_TEXT, idleInActiveStage, isCommentNews, isOwnStageChange, lifecycle, mechanicalStatus, namesUserAsBlocker, newHumanComments, newestCommentAt, openUserFacingItems, ownChecksFinished, ownPullClosed, ownPullMerged, phaseAndStage, productGateOptions, runningWorkerReason, newestEditComment, specDiff, specHash, specText, stuckDue, unheardWait, workerInFlight, workerReportedTerminal, type Change, type MechanicalEvent, type OpenUserItem } from "./reconcile.js"; import { classifyApproval, latestProjectApprovalMarker, latestReviewMarker, prGate, reviewMarker } from "./gate.js"; -import { approveOwnerReviewedPull, approvePrPageAction, declineOwnerReviewedPull, fileForwardedApprovePrNeed, isProjectSteward, isUserThread, markerKey, normalizeText, placeReportedPull, specApprovedByProject, syncPullApproval, userApprovedOnEntity, userSaid, type ApprovalsContext } from "./approvals.js"; +import { approveOwnerReviewedPull, approvePrPageAction, declineOwnerReviewedPull, fileForwardedApprovePrNeed, isProjectSteward, isUserThread, markerKey, normalizeText, placeReportedPull, recordProjectDecision, specApprovedByProject, syncPullApproval, userApprovedOnEntity, userSaid, type ApprovalsContext } from "./approvals.js"; import { deliverEscalation, nextRoute, routedTo, type EscalationsContext } from "./escalations.js"; import { approvalWanted, briefGate, gateState, resolveChecklistItem, syncApprovalRequest, type ReadyGateContext } from "./ready-gate.js"; import { @@ -120,7 +120,7 @@ import { import { DEPLOY_STATES, allDeployed, applyDeployment, applyDeploymentRead, applyWorkflowRun, awaitingOnRepo, deploymentLines, deploymentReportSchema, isFinalDeploymentState, matchesTarget, matchesWorkflow, mergedPulls, pickWatch, recordMerged, reportDeployment, targetKeyOf, timeOut, timeoutsDue } from "./deploy.js"; import { evaluateCarries, matchCarriesRule, noRuleNoted, type CarriesReader } from "./carries.js"; import { ACTORS, MIGRATIONS, ESCALATED_NEED_KINDS, NEEDS_KINDS, RESOLUTION_KINDS, createStore, needChoices, type Assumption, type Binding, type DepsDecision, type Escalation, type Fingerprint, type Lesson, type Need, type Observed, type Question, type ReadyState, type Store, type WakeRetry, type Worker } from "./store.js"; -import { hasLessonAnchor, lessonAnchor, planLessonTransitions, planValidations, retiredLessonAnchor, withLessonAnchor } from "./lessons.js"; +import { hasLessonAnchor, landLessonInstruction, lessonAnchor, lessonLandingMark, planLessonTransitions, planValidations, retiredLessonAnchor, withLessonAnchor } from "./lessons.js"; import { AFTER_THE_FACT_RATIONALE_WHY, REPEAT_RATIONALE_WHY, ROUTINE_RATIONALE_WHY, clusterKey, routineRationaleWanted } from "./rationale.js"; import { GUIDANCE_UNIT_PREFIX, GUIDANCE_UNCITED_DAYS, escalationWeeks, parseGuidanceUnits, uncitedGuidanceUnits, type GuidanceUnit } from "./guidance.js"; import { createGhWriter, parsePullRef, type BranchProtection, type GithubWriter, type GithubTreeEntry } from "./github.js"; @@ -1944,6 +1944,41 @@ export function createStewardsPlugin(deps: StewardsDeps): (bb: BbPluginApi) => P return value; } + /** The `server_settings` key that records when `gu_` ids were first rendered for a project (BB-288). */ + const guidanceRenderedKey = (projectId: string): string => `stewards.guidanceFirstRenderedAt.${projectId}`; + + /** When the plugin first rendered `gu_` ids for a project, or null when it has not (BB-288). */ + function guidanceFirstRenderedAt(projectId: string): number | null { + const raw = store.serverSetting(guidanceRenderedKey(projectId)); + if (raw === null) return null; + const at = Number(raw); + return Number.isFinite(at) ? at : null; + } + + /** Record the first `gu_` render, once, so the efficacy "uncited" list waits 90 days from it (BB-288). */ + function noteGuidanceRendered(projectId: string): void { + if (guidanceFirstRenderedAt(projectId) !== null) return; + store.setServerSetting(guidanceRenderedKey(projectId), String(now())); + } + + /** The AGENTS.md and playbook units a wake renders: `AGENTS.md` always, plus the binding's playbook (BB-288). */ + function guidanceFor(units: readonly GuidanceUnit[], playbookName: string | null): GuidanceUnit[] { + const file = playbookName === null ? null : playbookPath(playbookName); + return units.filter((u) => u.file === "AGENTS.md" || (file !== null && u.file === file)); + } + + /** The guidance units for a wake, read (and cached) from `.bb` HEAD, recording the first render (BB-288). */ + async function wakeGuidance(projectId: string, playbookName: string | null): Promise { + const units = await guidanceUnits(projectId); + noteGuidanceRendered(projectId); + return guidanceFor(units, playbookName); + } + + /** The last loaded guidance units for a brief's synchronous context, without a read (BB-288). */ + function cachedWakeGuidance(projectId: string, playbookName: string | null): GuidanceUnit[] { + return guidanceFor(guidanceIndexes.get(projectId)?.value ?? [], playbookName); + } + /** * Count the lessons and hand-written guidance units one decision or report * rested on (BB-35 steps 2 and 6). A lesson's `hits` counts distinct @@ -3433,6 +3468,7 @@ export function createStewardsPlugin(deps: StewardsDeps): (bb: BbPluginApi) => P deployments: b.deployments, assumptions: store.openAssumptions(b.id), lessons: store.lessonsByTier(b.projectId, "trial").slice(0, config.file.lessons.trial.max), + guidance: await wakeGuidance(b.projectId, playbook.name), now: now(), }); const delivery = await deliverWake(b, config, playbook, text); @@ -5753,7 +5789,7 @@ export function createStewardsPlugin(deps: StewardsDeps): (bb: BbPluginApi) => P const questions = store.openQuestions(projectId).map((question) => ({ question, about: question.bindingId === null ? null : store.binding(question.bindingId) })); const needs = open.flatMap((b) => store.openNeeds(b.id).map((need) => ({ need, binding: b }))); const assumptions = store.openAssumptionsForProject(projectId).map((assumption) => ({ assumption, binding: assumption.bindingId === null ? null : (store.binding(assumption.bindingId) ?? null) })); - const text = formatProjectWake({ reason, bindings, routed, withUser, problems: config.problems, digestDue: opts.digestDue, notes: opts.notes ?? [], bigDecisions: config.file.bigDecisions, standingRules: config.file.standingRules, questions, needs, assumptions, lessons: store.allLessons(projectId), lessonRules: config.file.lessons, timeZone: config.file.nightly.timezone, label: config.file.label, ...(records === undefined ? {} : { records }) }); + const text = formatProjectWake({ reason, bindings, routed, withUser, problems: config.problems, digestDue: opts.digestDue, notes: opts.notes ?? [], bigDecisions: config.file.bigDecisions, standingRules: config.file.standingRules, questions, needs, assumptions, lessons: store.allLessons(projectId), guidance: await wakeGuidance(projectId, null), lessonRules: config.file.lessons, timeZone: config.file.nightly.timezone, label: config.file.label, ...(records === undefined ? {} : { records }) }); const delivered = { ids: routed.filter((r) => r.escalation.deliveredAt === null).map((r) => r.escalation.id), at: 0 }; delivered.at = store.markDelivered(delivered.ids); // A wake can be the thing that pushes the transcript over the limit. @@ -5955,7 +5991,7 @@ export function createStewardsPlugin(deps: StewardsDeps): (bb: BbPluginApi) => P return null; } - /** Squash-merge a lesson's pull request as the project steward, and mark the lesson merged. The narrow guard is checked first, and a refusal is recorded as a note, like a refused open, so the nightly wake shows it. */ + /** Squash-merge a lesson's pull request as the project steward, and mark the lesson merged. The narrow guard is checked first, and a refusal is recorded as a note, like a refused open, so the nightly wake shows it. BB-288: the ordinary decide list places the landing; a `user` file is the owner's, not the project steward's. */ async function mergeLessonPr(lesson: Lesson, result: LessonPrResult, note: string, opts: { retire?: boolean; verb?: string } = {}): Promise<{ ok: boolean; message: string }> { if (result.repo === null || result.number === null) return { ok: true, message: `pull request opened at ${result.url ?? "?"}` }; const retire = opts.retire === true; @@ -5964,6 +6000,30 @@ export function createStewardsPlugin(deps: StewardsDeps): (bb: BbPluginApi) => P store.noteLesson(lesson.id, problem); return { ok: false, message: `pull request refused: ${problem}` }; } + // BB-288: the decide list decides who lands the lesson, exactly as it + // decides any other pull request. A `user` file (AGENTS.md, the project + // file, roles.json) files the owner's Review PR/Drop item; a `project` + // file (a playbook) is the project steward's, and the ordinary project + // approval is recorded before the merge. + const decisions = (await loadConfig(lesson.projectId))?.file.bigDecisions ?? []; + if (lessonLandingMark(lesson, decisions) === "user") { + const filed = fileLessonOwnerNeed(lesson, result.branch, result.url, retire); + const message = filed + ? "the decide list makes this the user's to approve; Review PR/Drop filed for the owner" + : "the decide list makes this the user's to approve, and the lesson has no binding to file it on"; + store.noteLesson(lesson.id, message); + return { ok: false, message }; + } + const binding = lesson.bindingId === null ? null : store.binding(lesson.bindingId); + const projectThreadId = store.project(lesson.projectId).threadId; + const pull = await snapshot("github", `${result.repo}#${result.number}`).catch(() => null); + if (binding !== null && projectThreadId !== null && pull !== null && (pull.pull?.headSha ?? "") !== "") { + const files = github.files === undefined ? null : await github.files(result.repo, result.number).catch(() => null); + const entry = files === null ? null : (explainDecision(files, decisions).winner?.text ?? null); + await recordProjectDecision(approvalsCtx, binding, pull, projectThreadId, { headSha: pull.pull?.headSha ?? "", entry, verdict: "approved", reasoning: note }).catch((error: unknown) => { + bb.log.warn(`stewards: could not record the project approval for lesson ${lesson.id}: ${errorMessage(error)}`); + }); + } try { const { sha } = await github.squashMerge(result.repo, result.number, `Lesson ${lesson.id}`, note); store.markLessonMerged(lesson.id, sha, result.branch ?? lesson.branch, note); @@ -6153,22 +6213,28 @@ export function createStewardsPlugin(deps: StewardsDeps): (bb: BbPluginApi) => P } /** - * File the boundary lesson's Review PR/Drop item (BB-35 steps 3/5, BB-108). - * It is the existing validation escalation: only the owner merging the pull - * request makes a boundary lesson permanent, and an unanswered item leaves - * the lesson in trial. + * File the owner's Review PR/Drop item for a lesson the decide list or a + * boundary scope makes the user's (BB-35 steps 3/5, BB-108; BB-288). The + * project steward publishes the branch and opens the pull request; the + * owner approves it on GitHub, and the plugin observes the merge. It is the + * existing validation escalation: only the owner merging the pull request + * makes a boundary lesson permanent, and an unanswered item leaves the + * lesson in trial. */ - function fileBoundaryLessonNeed(lesson: Lesson, branch: string | null, prUrl: string | null, retire = false): boolean { + function fileLessonOwnerNeed(lesson: Lesson, branch: string | null, prUrl: string | null, retire = false, boundary = false): boolean { if (lesson.bindingId === null) return false; + const kind = boundary ? "boundary lesson" : "lesson"; store.fileNeed({ bindingId: lesson.bindingId, kind: "decide", url: lesson.id, - what: `Review or drop the ${retire ? "retirement of the " : ""}boundary lesson for ${lesson.file}`, + what: `Review or drop the ${retire ? "retirement of the " : ""}${kind} for ${lesson.file}`, how: [ retire - ? "The nightly pass retired this boundary lesson, so removing its anchored text also changes the boundary and only you may make it permanent." - : "The project steward recorded a general rule that changes its own escalation boundary, so only you may make it permanent.", + ? `The nightly pass retired this ${kind}, so removing its anchored text ${boundary ? "also changes the boundary and " : ""}only you may make it permanent.` + : boundary + ? "The project steward recorded a general rule that changes its own escalation boundary, so only you may make it permanent." + : "The project steward recorded a general rule for a file the decide list makes the user's, so only you may make it permanent.", "", `> ${lesson.text}`, "", @@ -6181,7 +6247,9 @@ export function createStewardsPlugin(deps: StewardsDeps): (bb: BbPluginApi) => P return true; } - /** File the mixed-evidence Keep/Retire question (BB-35 step 5). Silence retires the lesson. */ + /** File the boundary lesson's Review PR/Drop item (BB-35 steps 3/5, BB-108). */ + const fileBoundaryLessonNeed = (lesson: Lesson, branch: string | null, prUrl: string | null, retire = false): boolean => fileLessonOwnerNeed(lesson, branch, prUrl, retire, true); + function fileMixedEvidenceNeed(lesson: Lesson): boolean { if (lesson.bindingId === null) return false; store.fileNeed({ @@ -9009,7 +9077,7 @@ export function createStewardsPlugin(deps: StewardsDeps): (bb: BbPluginApi) => P const hostId = cachedProjectHost(b.projectId); const flags = hostId === undefined ? null : [environmentFlags(config?.source ?? null, hostId), workerFlags(workerModel(b, playbook))].filter((f) => f !== "").join(" "); return formatBrief( - { binding: b, playbook, gate: briefGate(readyGateCtx, b, playbook), workerFlags: flags, escalations: store.openEscalations(b.id), conflicts: conflictsOf(b, store.open(b.projectId)) }, + { binding: b, playbook, gate: briefGate(readyGateCtx, b, playbook), workerFlags: flags, escalations: store.openEscalations(b.id), conflicts: conflictsOf(b, store.open(b.projectId)), guidance: cachedWakeGuidance(b.projectId, playbook.name) }, BRIEF_MAX_CHARS, ); }); @@ -10022,7 +10090,7 @@ export function createStewardsPlugin(deps: StewardsDeps): (bb: BbPluginApi) => P const cluster = lessonClusterFor(bindingId, escalationId); const lesson = store.addLesson({ projectId: projectOf!, bindingId, escalationId, file: input.lesson!.file, text: input.lesson!.text, scope: input.lesson!.scope, branch, ...(cluster === null ? {} : { clusterKey: cluster }), ...(input.lesson!.supersedes === undefined ? {} : { supersedes: input.lesson!.supersedes }) }); if (cluster !== null) store.settleClusterRationale(projectOf!, cluster); - return `lesson ${lesson.id} recorded (${lesson.scope}, ${lesson.tier}); it is injected into wakes and the nightly pass promotes it when it earns it. To land it now, add \`${withLessonAnchor(lesson.text, lesson.id)}\` to ${lesson.file}, commit, publish it with workspace_publish_guidance on a multi-repo project (on a native project, commit and open the pull request yourself), then complete it with steward_update { lessonAction: { id: "${lesson.id}", action: "complete" } }`; + return `lesson ${lesson.id} recorded (${lesson.scope}, ${lesson.tier}); it is injected into wakes and the nightly pass promotes it when it earns it. ${landLessonInstruction(lesson)}`; }; if (projectOf !== null && resolve?.startsWith("qst_") === true) { const q = store.question(resolve); @@ -11490,7 +11558,7 @@ export function createStewardsPlugin(deps: StewardsDeps): (bb: BbPluginApi) => P if (config === null || playbook === null) throw new PluginCliError(`No playbook for ${b.entityKey}: run bb stewards check.`, { code: "playbook_missing" }); return { exitCode: 0, - stdout: formatBrief({ binding: b, playbook, gate: briefGate(readyGateCtx, b, playbook), workerFlags: await workerFlagsFor(b, playbook, config.source), escalations: store.openEscalations(b.id), conflicts: conflictsOf(b, store.open(b.projectId)) }), + stdout: formatBrief({ binding: b, playbook, gate: briefGate(readyGateCtx, b, playbook), workerFlags: await workerFlagsFor(b, playbook, config.source), escalations: store.openEscalations(b.id), conflicts: conflictsOf(b, store.open(b.projectId)), guidance: await wakeGuidance(b.projectId, playbook.name) }), }; }, }), @@ -11637,7 +11705,16 @@ export function createStewardsPlugin(deps: StewardsDeps): (bb: BbPluginApi) => P // guidance nothing has cited in 90 days. Read-only: it names a // suggestion, it never removes a unit. const units = await guidanceUnits(projectId); - const uncited = uncitedGuidanceUnits(units, store.guidanceCitations(projectId), now()); + // BB-288: the "uncited for 90 days" list is suppressed until + // `gu_` ids have actually been rendered for 90 days. A unit + // reads "never cited" because no wake ever showed its id, not + // because nobody followed it; the stored first-render date is + // when that stopped being true. Read-only: this command never + // sets the date. + const firstRenderedAt = guidanceFirstRenderedAt(projectId); + const uncitedReadyAt = firstRenderedAt === null ? null : firstRenderedAt + GUIDANCE_UNCITED_DAYS * 86_400_000; + const uncitedReady = uncitedReadyAt !== null && now() >= uncitedReadyAt; + const uncited = uncitedReady ? uncitedGuidanceUnits(units, store.guidanceCitations(projectId), now()) : []; // One count per user-facing item: a forwarded escalation is // represented by the question the user saw, not both. The // question's creation is when it reached them. @@ -11647,8 +11724,15 @@ export function createStewardsPlugin(deps: StewardsDeps): (bb: BbPluginApi) => P const weeks = escalationWeeks(reaching.map((x) => x.createdAt), now()); if (input.options.json) return { exitCode: 0, stdout: JSON.stringify({ weeks, uncited: uncited.map((u) => ({ id: u.unit.id, file: u.unit.file, heading: u.unit.heading, text: u.unit.text, lastCitedAt: u.lastCitedAt })) }) }; const lines = ["Escalations reaching the user, per week (oldest first):", ...weeks.map((w) => ` ${iso(w.start).slice(0, 10)} ${w.count}`)]; - lines.push("", `Hand-written guidance units uncited for ${GUIDANCE_UNCITED_DAYS} days (consider removing):`); - if (uncited.length === 0) lines.push(" none"); + if (uncitedReady) lines.push("", `Hand-written guidance units uncited for ${GUIDANCE_UNCITED_DAYS} days (consider removing):`); + else + lines.push( + "", + `Hand-written guidance units uncited for ${GUIDANCE_UNCITED_DAYS} days: suppressed ${ + uncitedReadyAt === null ? "until the ids have been rendered in a wake" : `until ${iso(uncitedReadyAt).slice(0, 10)}` + }, because \`gu_\` ids have not been rendering for ${GUIDANCE_UNCITED_DAYS} days yet.`, + ); + if (uncitedReady && uncited.length === 0) lines.push(" none"); for (const u of uncited) { const snippet = u.unit.text.replace(/\s+/gu, " ").slice(0, 120); lines.push(` ${u.unit.id} ${u.unit.file}${u.unit.heading === null ? "" : ` §${u.unit.heading}`} ${u.lastCitedAt === null ? "(never cited)" : `(last cited ${iso(u.lastCitedAt).slice(0, 10)})`}\n ${snippet}`); diff --git a/plugins/stewards/skills/stewards/SKILL.md b/plugins/stewards/skills/stewards/SKILL.md index 2eea2e04..0a6c3beb 100644 --- a/plugins/stewards/skills/stewards/SKILL.md +++ b/plugins/stewards/skills/stewards/SKILL.md @@ -673,9 +673,11 @@ Because `.bb` is a directory on the primary repo's default branch, a lesson lands as a pull request, never a fast-forward: the plugin opens one PR per lesson (or per publish when several lessons touch the same file) on the primary repo, naming the lesson id, the anchor and you as the proposing -steward. A `procedure` lesson's PR is squash-merged by the project steward at -once, with no person asked; a `boundary` lesson's PR waits for the owner to -merge it, and a project-steward approval never counts for it. A lesson may +steward. The decide list places that pull request like any other: a +`procedure` lesson in a `project` file (a playbook) is squash-merged by the +project steward at once, with no person asked; a lesson in a `user` file +(`AGENTS.md`) or a `boundary` lesson waits for the owner as a Review PR/Drop +item, and a project-steward approval never counts for it. A lesson may never target `stewards/project.json` or `stewards/roles.json`, so it cannot widen the decide list, loosen the pull-request gate, relax an approval or change who is a steward; a boundary lesson still goes to the owner as diff --git a/plugins/stewards/wake.test.ts b/plugins/stewards/wake.test.ts index 4421561b..d9bb6c2e 100644 --- a/plugins/stewards/wake.test.ts +++ b/plugins/stewards/wake.test.ts @@ -367,6 +367,40 @@ describe("the nightly prompt", () => { expect(wake).not.toContain(`- ${lessons[2]!.id} `); }); + it("tells the project steward it may land a trial lesson early, and that it is not an owner priority (BB-288)", () => { + const { s, bind } = world(); + const b = bind("ENG-12"); + const trial = s.addLesson({ projectId: "p1", bindingId: b.id, escalationId: null, file: "AGENTS.md", text: "Auth is the plugin's.", scope: "procedure", branch: null }); + const wake = formatProjectWake({ reason: "nightly", bindings: [], routed: [], problems: [], digestDue: true, notes: [], bigDecisions: [], questions: [], lessons: s.allLessons("p1") }); + expect(wake).toContain("## Lessons (trial)"); + expect(wake).toContain("not owner to-dos"); + expect(wake).toContain("workspace_publish_guidance"); + expect(wake).toContain(`steward_update { lessonAction: { id: "${trial.id}", action: "complete" } }`); + // The digest is built from the records, which never carry trial lessons, + // so a trial rule cannot be listed as an owner priority (BB-288). + const records: DigestRecords = { previous: null, since: T, moved: [], ended: [], answered: [], withUser: [], stuck: [], noWakeSource: [], stall: [], held: [] }; + const digest = formatRecords(records, []).join("\n"); + expect(digest).not.toContain(trial.id); + expect(digest).not.toContain("Auth is the plugin's."); + }); + + it("renders each AGENTS.md and playbook guidance unit beside its gu_ id in the wake and the brief (BB-288)", () => { + const units = [ + { id: "gu_0123456789abcdef", file: "AGENTS.md", heading: "Reviews", text: "A merge to bb-plugins main restarts every server.", lessonId: null }, + { id: "gu_fedcba9876543210", file: "stewards/playbooks/linear-tickets.md", heading: "In Progress", text: "Start the worker in the same turn.", lessonId: null }, + ]; + const input = wakeInput({ guidance: units }); + const wake = formatWake(input); + expect(wake).toContain("## Guidance units"); + expect(wake).toContain("- gu_0123456789abcdef (AGENTS.md §Reviews): A merge to bb-plugins main restarts every server."); + expect(wake).toContain("- gu_fedcba9876543210 (stewards/playbooks/linear-tickets.md §In Progress): Start the worker in the same turn."); + const brief = formatBrief({ binding: input.binding, playbook: input.playbook, gate: input.gate, workerFlags: null, guidance: units }); + expect(brief).toContain("gu_0123456789abcdef"); + expect(brief).toContain("gu_fedcba9876543210"); + // With no units the block is absent, so a wake that renders none says nothing. + expect(formatWake(wakeInput())).not.toContain("## Guidance units"); + }); + it("names the five resolution kinds and how a credential is forwarded (BB-51, BB-176)", () => { const { s, bind } = world(); const b = bind("ENG-12"); diff --git a/plugins/stewards/wake.ts b/plugins/stewards/wake.ts index 685e2334..5b7557a4 100644 --- a/plugins/stewards/wake.ts +++ b/plugins/stewards/wake.ts @@ -6,6 +6,8 @@ // it ever saw can act correctly from one wake alone, which is what lets its // transcript stop being the source of truth. import { formatDuration, GITHUB_TODO_LABEL, INITIATIVE_LABEL, INITIATIVE_PLAYBOOK, phaseInstructions, type FilingTarget, type LessonRules, type Playbook } from "./config.js"; +import { landLessonInstruction } from "./lessons.js"; +import type { GuidanceUnit } from "./guidance.js"; import type { BigDecision } from "./decisions.js"; import { anyPending, type Deployments, type DeploymentWatch } from "./deploy.js"; import { localParts } from "./nightly.js"; @@ -326,6 +328,8 @@ export type WakeInput = { conflicts?: Conflict[]; /** Trial lessons of the project, already capped to `lessons.trial.max`; candidates never appear. */ lessons?: Lesson[]; + /** The hand-written guidance units this wake renders, each with the `gu_` id a steward cites (BB-288). */ + guidance?: GuidanceUnit[]; now: number; }; @@ -631,6 +635,7 @@ export function formatWake(input: WakeInput): string { if (binding.mergeBehind.length > 0) out.push(mergeHoldLine(binding)); if (binding.stackedPulls.length > 0) out.push(stackedPullsLine(binding)); out.push(...schedulingLines(binding, input.conflicts ?? [], input.escalations)); + out.push(...guidanceUnitLines(input.guidance ?? [])); out.push(...trialLessonsLines(input.lessons ?? [])); out.push(...assumptionLines(input.assumptions ?? [])); // BB-154: a wait with no structured source gets one paragraph, before the @@ -697,6 +702,8 @@ export type BriefInput = { escalations?: Escalation[]; /** Open bindings at work on the same paths, for the scheduling line. */ conflicts?: Conflict[]; + /** The hand-written guidance units this brief renders, each with the `gu_` id a steward cites (BB-288). */ + guidance?: GuidanceUnit[]; }; /** One line for the head of a brief: the gate's state, and the one action it forbids while unmet. */ @@ -753,6 +760,7 @@ export function formatBrief(input: BriefInput, maxChars?: number): string { middle.push(`- Priority: ${b.priority ?? "normal"}`); middle.push(holdLine(b, "take no new actions and start no workers until it is lifted.")); middle.push(...schedulingLines(b, input.conflicts ?? [], escalations)); + middle.push(...guidanceUnitLines(input.guidance ?? [])); const tail: string[] = [ "", @@ -1213,6 +1221,23 @@ function trialLessonsLines(lessons: Lesson[]): string[] { ]; } +/** + * The hand-written guidance units a wake or brief renders, each beside its + * `gu_` id (BB-288). Without the id a steward can only cite a unit by running + * `bb stewards lessons --efficacy`, so every unit the wake shows carries the + * id it must cite. + */ +function guidanceUnitLines(units: readonly GuidanceUnit[]): string[] { + if (units.length === 0) return []; + return [ + "", + "## Guidance units", + "", + "Each hand-written paragraph of `AGENTS.md` and the playbooks carries its `gu_` id. Cite the id of the paragraph a decision or turn rested on in `steward_update { resolve, cites: [...] }` (or `{ forward, cites }`) or `steward_report { cites: [...] }`.", + ...units.map((u) => `- ${u.id} (${u.file}${u.heading === null ? "" : ` §${u.heading}`}): ${oneLine(u.text)}`), + ]; +} + /** * The project steward's lesson blocks (BB-35 step 3): the trial lessons to * decide from, the promoted ones the nightly pass queued for landing, and the @@ -1224,8 +1249,13 @@ function projectLessonsLines(lessons: Lesson[], rules: LessonRules | undefined): const out: string[] = []; const trial = lessons.filter((l) => l.tier === "trial").slice(0, cap); if (trial.length > 0) { - out.push("", "## Lessons (trial)", "", "Project rules not yet in `.bb`. Decide from them, and cite the ones a decision rested on with `steward_update { resolve, cites: [lesson ids] }` (or `{ forward, cites }`), or the ones a turn acted on in `steward_report { cites: [lesson ids] }`. A decision from hand-written guidance may cite its unit id (`gu_…`) the same way."); - for (const l of trial) out.push(`- ${l.id} (${l.scope}, ${l.file}): ${oneLine(l.text)}`); + out.push( + "", + "## Lessons (trial)", + "", + "Project rules not yet in `.bb`. These are yours to land early or leave in trial; they are not owner to-dos and never belong under the digest's \"today's priorities\". Decide from them, and cite the ones a decision rested on with `steward_update { resolve, cites: [lesson ids] }` (or `{ forward, cites }`), or the ones a turn acted on in `steward_report { cites: [lesson ids] }`. A decision from hand-written guidance may cite its unit id (`gu_…`) the same way.", + ); + for (const l of trial) out.push(`- ${l.id} (${l.scope}, ${l.file}): ${oneLine(l.text)}\n ${landLessonInstruction(l)}`); } const toLand = lessons.filter((l) => l.tier === "promoted" && l.status === "pending"); if (toLand.length > 0) { @@ -1233,7 +1263,7 @@ function projectLessonsLines(lessons: Lesson[], rules: LessonRules | undefined): "", "## Lessons to land", "", - 'The nightly pass promoted these. Add the anchored block to the named file in your `.bb` checkout, publish with workspace_publish_guidance, then complete it with steward_update { lessonAction: { id: "", action: "complete" } }. The plugin opens one pull request on the primary repo per lesson (or per publish when several touch the same file): a `procedure` one is squash-merged by the project steward, and a `boundary` one is filed for the owner as a Review PR/Drop item that waits for the owner to merge it. If the pull request was refused, the reason is on the lesson: fix the branch and complete it again, or drop it.', + 'The nightly pass promoted these. Add the anchored block to the named file in your `.bb` checkout, publish with workspace_publish_guidance, then complete it with steward_update { lessonAction: { id: "", action: "complete" } }. The plugin opens one pull request on the primary repo per lesson (or per publish when several touch the same file). The decide list places it like any other pull request: a `project` file (a playbook) is squash-merged by the project steward, while a `user` file (`AGENTS.md`) or a `boundary` lesson is filed for the owner as a Review PR/Drop item that waits for the owner to merge it. If the pull request was refused, the reason is on the lesson: fix the branch and complete it again, or drop it.', ); for (const l of toLand) out.push(`- ${l.id} (${l.scope}, ${l.file}): ${oneLine(l.text)}\n Anchor: \`\`${l.note === null ? "" : `\n Last attempt: ${oneLine(l.note)}`}`); } @@ -1243,7 +1273,7 @@ function projectLessonsLines(lessons: Lesson[], rules: LessonRules | undefined): "", "## Lessons to retire", "", - 'The nightly pass retired these. In your `.bb` checkout replace the lesson\'s anchored block (its `` line and the wording under it) with the retired anchor ``, publish with workspace_publish_guidance, then retire it with steward_update { lessonAction: { id: "", action: "retire" } }. A retire is a pull request too: a `procedure` one is merged by the project steward, a `boundary` one waits for the owner. The row is kept.', + 'The nightly pass retired these. In your `.bb` checkout replace the lesson\'s anchored block (its `` line and the wording under it) with the retired anchor ``, publish with workspace_publish_guidance, then retire it with steward_update { lessonAction: { id: "", action: "retire" } }. A retire is a pull request too: the decide list places it the same way — a `project` playbook is merged by the project steward, a `user` file or a `boundary` lesson waits for the owner. The row is kept.', ); for (const l of toRetire) out.push(`- ${l.id} (${l.file}): ${oneLine(l.text)}`); } @@ -1382,6 +1412,8 @@ export function formatProjectWake(input: { assumptions?: Array<{ assumption: Assumption; binding: Binding | null }>; /** Pending lessons: general rules waiting to be landed in `.bb`. */ lessons?: Lesson[]; + /** The hand-written guidance units this wake renders, each with the `gu_` id a steward cites (BB-288). */ + guidance?: GuidanceUnit[]; /** The project's lesson rules, for the trial cap and the wake headings. */ lessonRules?: LessonRules; /** With digestDue: the records the digest is written from. */ @@ -1487,6 +1519,7 @@ export function formatProjectWake(input: { out.push("", "Recorded, not asked: the owner sees these here and can correct one; a confirmation is the steward's own. Do not ask the user about one in prose.", ""); } const lessons = input.lessons ?? []; + out.push(...guidanceUnitLines(input.guidance ?? [])); out.push(...projectLessonsLines(lessons, input.lessonRules)); const head = out.splice(0); out.push(`## Bindings (${input.bindings.length} open)`, ""); From 27c00b3992ae6ca9ff1537fa61db7e74ffaf2ca8 Mon Sep 17 00:00:00 2001 From: Dylan Trotter Date: Tue, 10 Nov 2026 12:11:35 +0000 Subject: [PATCH 2/4] stewards: address review round 1 (BB-288) - Render the guidance index compactly (id, file, heading, a 120-char snippet, capped at 60 units) and place it where `fit` can clip it, so it can never push the wake past its cap or cost the snapshot and "What changed". Add a wake test with 300 units asserting both survive. - Make the lesson e2e serve the pull request's snapshot and assert the ordinary project approval names the BB-280 playbook entry and is posted before the merge, so the path cannot silently skip. --- plugins/stewards/server.test.ts | 16 +++++++++++++++- plugins/stewards/wake.test.ts | 22 +++++++++++++++++++++- plugins/stewards/wake.ts | 22 ++++++++++++++++++---- 3 files changed, 54 insertions(+), 6 deletions(-) diff --git a/plugins/stewards/server.test.ts b/plugins/stewards/server.test.ts index 0196443f..ad9265d3 100644 --- a/plugins/stewards/server.test.ts +++ b/plugins/stewards/server.test.ts @@ -122,6 +122,8 @@ function world() { const userTurns: Array<{ threadId: string; text: string; at?: number }> = []; /** What the page's GitHub actions did. */ const githubComments: Array<{ repo: string; number: number; body: string }> = []; + /** The order of the github writer's comment/merge calls, so a test can assert a marker preceded a merge (BB-288). */ + const githubCallOrder: string[] = []; const githubReviews: Array<{ repo: string; number: number; event: string; body: string }> = []; const githubMerges: Array<{ repo: string; number: number; title: string; body: string }> = []; /** Workflow dispatches the fake github serves, in order (BB-216). */ @@ -152,6 +154,7 @@ function world() { const github = { comment: async (repo: string, number: number, body: string) => { githubComments.push({ repo, number, body }); + githubCallOrder.push("comment"); return `IC_${githubComments.length}`; }, review: async (repo: string, number: number, event: "APPROVE" | "REQUEST_CHANGES" | "COMMENT", body: string) => { @@ -167,6 +170,7 @@ function world() { throw new Error(`PUT /repos/${repo}/pulls/${number}/merge failed: 409 {"message":"Head branch was modified. Review and try the merge again."}`); } githubMerges.push({ repo, number, title, body }); + githubCallOrder.push("merge"); // The real merge changes the pull request's state; the fake must too, or // the plugin's own gate-met merge (BB-257) and the reconcile it // schedules would merge the same open snapshot again in a loop. @@ -563,7 +567,7 @@ function world() { }, }; let now = Date.parse("2026-10-14T15:00:00Z"); - return { files, writes, writtenContent, fileContents, repos, primary, entities, projectList, dependabotAlerts, dependabotGraphEmpty, sourceDeployments, sourceFiles, sourceCommits, sourceRuns, sourceCompares, labelEnsureCalls, labelsEnsured, labelEnsureFailures, setLabelEnsureUnavailable: (value: boolean) => { labelEnsureUnavailable = value; }, setPushUnavailable: (value: boolean) => { pushUnavailable = value; }, setLabelEnsureThrow: (value: string | null) => { labelEnsureThrow = value; }, setLayout: (value: "primary" | "directory") => { layout = value; primary = value === "directory" ? null : { dir: "bb", url: "https://github.com/acme/bb.git" }; }, viewers, setLinearMine: (teams: Array<{ unit: string; name: string }>) => { linearMine = teams; }, setMissingStates: (source: "linear" | "github", unit: string, states: string[]) => { missingStates.set(`${source}:${unit.toLowerCase()}`, states); }, setMissingLabel: (source: "linear" | "github", unit: string, labels: string[]) => { missingLabels.set(`${source}:${unit.toLowerCase()}`, labels); }, rpcCalls, githubTrees, githubBranchProtection, githubLabels, setSnapshotError: (id: string, fail: boolean) => { if (fail) snapshotErrors.add(id); else snapshotErrors.delete(id); }, setSearchUnsupported: (value: boolean) => { searchUnsupported = value; }, setSearchError: (value: string | null) => { searchError = value; }, setCreatePullError: (value: string | null) => { createPullError = value; }, setMergePullError: (value: string | null) => { mergePullError = value; }, setMoveHeadOnMerge: (value: boolean) => { moveHeadOnMerge = value; }, setPullDetailsError: (value: string | null) => { pullDetailsError = value; }, setRetryConflict: (value: boolean) => { retryConflict = value; }, setRetryQueued: (value: boolean) => { retryQueued = value; }, threads, deletes, sections, usage, metadata, spawns, sends, retries, retryEvents, queuedDeletes, userTurns, githubComments, githubReviews, githubMerges, githubPulls, githubClosedPulls, githubDefaultBranches, githubBranches, githubFiles, githubFileContents, githubDispatches, setMergeDispatchError: (value: string | null) => { mergeDispatchError = value; }, github, sdk, uiPreferences, uiPreferencesWrites, now: () => now, advance: (ms: number) => (now += ms) }; + return { files, writes, writtenContent, fileContents, repos, primary, entities, projectList, dependabotAlerts, dependabotGraphEmpty, sourceDeployments, sourceFiles, sourceCommits, sourceRuns, sourceCompares, labelEnsureCalls, labelsEnsured, labelEnsureFailures, setLabelEnsureUnavailable: (value: boolean) => { labelEnsureUnavailable = value; }, setPushUnavailable: (value: boolean) => { pushUnavailable = value; }, setLabelEnsureThrow: (value: string | null) => { labelEnsureThrow = value; }, setLayout: (value: "primary" | "directory") => { layout = value; primary = value === "directory" ? null : { dir: "bb", url: "https://github.com/acme/bb.git" }; }, viewers, setLinearMine: (teams: Array<{ unit: string; name: string }>) => { linearMine = teams; }, setMissingStates: (source: "linear" | "github", unit: string, states: string[]) => { missingStates.set(`${source}:${unit.toLowerCase()}`, states); }, setMissingLabel: (source: "linear" | "github", unit: string, labels: string[]) => { missingLabels.set(`${source}:${unit.toLowerCase()}`, labels); }, rpcCalls, githubTrees, githubBranchProtection, githubLabels, setSnapshotError: (id: string, fail: boolean) => { if (fail) snapshotErrors.add(id); else snapshotErrors.delete(id); }, setSearchUnsupported: (value: boolean) => { searchUnsupported = value; }, setSearchError: (value: string | null) => { searchError = value; }, setCreatePullError: (value: string | null) => { createPullError = value; }, setMergePullError: (value: string | null) => { mergePullError = value; }, setMoveHeadOnMerge: (value: boolean) => { moveHeadOnMerge = value; }, setPullDetailsError: (value: string | null) => { pullDetailsError = value; }, setRetryConflict: (value: boolean) => { retryConflict = value; }, setRetryQueued: (value: boolean) => { retryQueued = value; }, threads, deletes, sections, usage, metadata, spawns, sends, retries, retryEvents, queuedDeletes, userTurns, githubComments, githubReviews, githubMerges, githubCallOrder, githubPulls, githubClosedPulls, githubDefaultBranches, githubBranches, githubFiles, githubFileContents, githubDispatches, setMergeDispatchError: (value: string | null) => { mergeDispatchError = value; }, github, sdk, uiPreferences, uiPreferencesWrites, now: () => now, advance: (ms: number) => (now += ms) }; } async function setup(extra: Partial = {}, settings: Record = {}, seedProvider?: string | null | "fail-list" | "fail-set", seedNavigation?: string | null | "fail-list" | "fail-set") { @@ -10603,6 +10607,9 @@ describe("lessons", () => { const t = await adopted(); const recorded = await t.tool("steward_update", { binding: "ENG-12", lesson: { text: lesson, file: LESSON_PLAYBOOK_FILE, scope: "procedure" } }, t.project); const id = /(lsn_[\w-]+)/u.exec(recorded.text)![1]!; + // The lesson's pull request is observable, so the ordinary project + // approval is recorded before the merge (BB-288). + t.entities.set("acme/bb#1", pull({ id: "acme/bb#1", key: "acme/bb#1", url: "https://github.com/acme/bb/pull/1" }, { headSha: "a".repeat(40), base: "main" })); const done = await t.tool("steward_update", { lessonAction: { id, action: "complete" } }, t.project); expect(done).toMatchObject({ error: false, text: expect.stringMatching(new RegExp(`${id}: merged at c{8}`, "u")) }); // Exactly one PR, on the primary repo, against its default branch, from the @@ -10612,6 +10619,13 @@ describe("lessons", () => { expect(pr).toMatchObject({ repo: "acme/bb", head: `guidance-${t.project}`, base: "main", title: expect.stringContaining(id) }); expect(pr.body).toContain(withLessonAnchor(lesson, id)); expect(pr.body).toContain(`_— from bb thread ${t.project}_`); + // The ordinary project approval names the BB-280 playbook entry and is + // posted before the merge. + const marker = t.githubComments.find((c) => c.body.includes("bb-approval")); + expect(marker?.body).toContain('entry="What every steward is told (the playbooks and the project\'s skills)"'); + const mergeAt = t.githubCallOrder.indexOf("merge"); + expect(mergeAt).toBeGreaterThan(-1); + expect(t.githubCallOrder.slice(0, mergeAt)).toContain("comment"); // No person is asked: the project steward squash-merges it. expect(t.githubMerges).toHaveLength(1); expect(t.hooks.store.lesson(id)).toMatchObject({ status: "merged", tier: "promoted", mergeSha: "c".repeat(40), branch: `refs/heads/guidance-${t.project}`, pr: pr.url }); diff --git a/plugins/stewards/wake.test.ts b/plugins/stewards/wake.test.ts index d9bb6c2e..c5dd19f8 100644 --- a/plugins/stewards/wake.test.ts +++ b/plugins/stewards/wake.test.ts @@ -7,7 +7,7 @@ import { comment, issue, pull } from "./fixtures.js"; import { gateState } from "./ready-gate.js"; import { MIGRATIONS, RESOLUTION_KINDS, createStore } from "./store.js"; import { DEPS_PRS, GITHUB_PRS, GITHUB_REVIEWS, LINEAR_TICKETS, OPS_WATCH, DEPS_ALERTS, PLAYBOOK_TEMPLATES } from "./templates.js"; -import { STANDING_RULES, BRIEF_PREFIX, formatBrief, formatDepsWake, formatOpsWake, formatProjectWake, formatRecords, formatWake, initiativeProgress, initiativeProgressFlag, spawnCommand, workerFinishNote, workerQuestion, type DigestRecords, type WakeInput } from "./wake.js"; +import { STANDING_RULES, BRIEF_PREFIX, formatBrief, formatDepsWake, formatOpsWake, formatProjectWake, formatRecords, formatWake, GUIDANCE_UNITS_MAX, initiativeProgress, initiativeProgressFlag, spawnCommand, WAKE_MAX_CHARS, workerFinishNote, workerQuestion, type DigestRecords, type WakeInput } from "./wake.js"; import { BRIEF_MAX_CHARS } from "./server.js"; import type { OpsFinding } from "./ops.js"; import type { DepsFinding } from "./deps.js"; @@ -401,6 +401,26 @@ describe("the nightly prompt", () => { expect(formatWake(wakeInput())).not.toContain("## Guidance units"); }); + it("keeps the snapshot and What changed when the guidance index is large (BB-288)", () => { + // A realistically sized AGENTS.md: far more units than a wake lists, each + // with a long paragraph. The compact, capped, clippable block must not + // push the wake past its cap or cost the snapshot or the changes. + const units = Array.from({ length: 300 }, (_, i) => ({ + id: `gu_${String(i).padStart(16, "0")}`, + file: "AGENTS.md", + heading: `Section ${i}`, + text: `Guidance paragraph number ${i}. `.repeat(40), + lessonId: null, + })); + const wake = formatWake(wakeInput({ guidance: units, changes: ["Something changed."] })); + expect(wake).toContain("## Guidance units"); + expect(wake).toContain(`and ${units.length - GUIDANCE_UNITS_MAX} more`); + expect(wake).toContain("## 4. Snapshot"); + expect(wake).toContain("## 5. What changed since your last turn"); + expect(wake).toContain("- Something changed."); + expect(wake.length).toBeLessThanOrEqual(WAKE_MAX_CHARS + 200); + }); + it("names the five resolution kinds and how a credential is forwarded (BB-51, BB-176)", () => { const { s, bind } = world(); const b = bind("ENG-12"); diff --git a/plugins/stewards/wake.ts b/plugins/stewards/wake.ts index 5b7557a4..88ead369 100644 --- a/plugins/stewards/wake.ts +++ b/plugins/stewards/wake.ts @@ -25,6 +25,11 @@ import { needChoices, ESCALATED_NEED_KINDS, RESOLUTION_KINDS, STEWARD_NEED_KINDS /** A wake longer than this is cut in its snapshot, never in what changed or what to do. */ export const WAKE_MAX_CHARS = 30_000; +/** How many guidance units a wake or brief lists before it says how many it left out (BB-288). */ +export const GUIDANCE_UNITS_MAX = 60; +/** The characters of a unit's own text a wake or brief shows; the `gu_` id is what is cited (BB-288). */ +const GUIDANCE_UNIT_CHARS = 120; + /** * What a steward must never ask for: an open escalation or approval request is * already on the user's Stewards page, and repeating it in a reply is noise. @@ -635,7 +640,6 @@ export function formatWake(input: WakeInput): string { if (binding.mergeBehind.length > 0) out.push(mergeHoldLine(binding)); if (binding.stackedPulls.length > 0) out.push(stackedPullsLine(binding)); out.push(...schedulingLines(binding, input.conflicts ?? [], input.escalations)); - out.push(...guidanceUnitLines(input.guidance ?? [])); out.push(...trialLessonsLines(input.lessons ?? [])); out.push(...assumptionLines(input.assumptions ?? [])); // BB-154: a wait with no structured source gets one paragraph, before the @@ -676,6 +680,9 @@ export function formatWake(input: WakeInput): string { } out.push("", OPEN_REQUESTS_NOTE, "Asking for one that is already open counts like a missed report toward `stuck`."); } + // BB-288: the guidance index sits at the end of the snapshot, so `fit` can + // clip it and never the snapshot or "What changed". + out.push(...guidanceUnitLines(input.guidance ?? [])); const snapshot = out.splice(0); out.push("", "## 5. What changed since your last turn", ""); @@ -1225,16 +1232,21 @@ function trialLessonsLines(lessons: Lesson[]): string[] { * The hand-written guidance units a wake or brief renders, each beside its * `gu_` id (BB-288). Without the id a steward can only cite a unit by running * `bb stewards lessons --efficacy`, so every unit the wake shows carries the - * id it must cite. + * id it must cite. The text is a short snippet, not the paragraph: the whole + * of `AGENTS.md` is already loaded natively, and a full-text index overflows + * the wake's cap. The block is placed where `fit` can clip it, so it can never + * cost the snapshot or "What changed". */ function guidanceUnitLines(units: readonly GuidanceUnit[]): string[] { if (units.length === 0) return []; + const shown = units.slice(0, GUIDANCE_UNITS_MAX); return [ "", "## Guidance units", "", "Each hand-written paragraph of `AGENTS.md` and the playbooks carries its `gu_` id. Cite the id of the paragraph a decision or turn rested on in `steward_update { resolve, cites: [...] }` (or `{ forward, cites }`) or `steward_report { cites: [...] }`.", - ...units.map((u) => `- ${u.id} (${u.file}${u.heading === null ? "" : ` §${u.heading}`}): ${oneLine(u.text)}`), + ...shown.map((u) => `- ${u.id} (${u.file}${u.heading === null ? "" : ` §${u.heading}`}): ${oneLine(u.text, GUIDANCE_UNIT_CHARS)}`), + ...(units.length > shown.length ? [`- … and ${units.length - shown.length} more; run \`bb stewards lessons --efficacy\` for the rest.`] : []), ]; } @@ -1519,7 +1531,6 @@ export function formatProjectWake(input: { out.push("", "Recorded, not asked: the owner sees these here and can correct one; a confirmation is the steward's own. Do not ask the user about one in prose.", ""); } const lessons = input.lessons ?? []; - out.push(...guidanceUnitLines(input.guidance ?? [])); out.push(...projectLessonsLines(lessons, input.lessonRules)); const head = out.splice(0); out.push(`## Bindings (${input.bindings.length} open)`, ""); @@ -1530,6 +1541,9 @@ export function formatProjectWake(input: { const flags = [initiativeProgressFlag(b), b.priority === null ? null : `priority ${b.priority}`, b.hold === null ? null : `HOLD: ${b.hold}${b.holdBehind.length === 0 ? "" : ` (behind ${b.holdBehind.join(", ")})`}${b.holdStack.length === 0 ? "" : ` (stacked on ${b.holdStack.join(", ")})`}`, b.touches.length === 0 ? null : `touches ${b.touches.join(", ")}`, conflicts.length === 0 ? null : `file-level overlap: ${conflicts.map((c) => { const open = c.with.lastSnapshot === null ? null : openPull(c.with, c.with.lastSnapshot); return `${c.with.entityKey} on ${c.on.join(", ")}${open === null || open.head === null || open.head === "" ? "" : ` (its pull request ${open.key} is open on ${open.head})`}`; }).join("; ")}`, openEscalations.length > 0 ? `${openEscalations.length} escalation(s): ${openEscalations.map((x) => `${x.kind}→${x.route}`).join(", ")}` : null, unheardWait(s, openEscalations)].filter((f): f is string => f !== null); out.push(`- ${b.entityKey} (${b.kind}, ${b.phase ?? "?"}): ${s === null ? "no report yet" : `${s.summary} Next: ${s.nextAction} (${s.actor})${s.waitingOn === null ? "" : `; waiting on ${s.waitingOn}${b.requeueUntil === null ? "" : ` until ${day(b.requeueUntil)}`}`}`}${flags.length > 0 ? ` [${flags.join("; ")}]` : ""}`); } + // BB-288: the guidance index sits at the end of the bindings list, so `fit` + // can clip it and never the bindings or the digest. + out.push(...guidanceUnitLines(input.guidance ?? [])); const bindings = out.splice(0); if (input.digestDue) { if (input.records !== undefined) out.push("", ...formatRecords(input.records, questions, input.timeZone)); From af5f207642e4f81b6fcf593edf92b4ee421b8da8 Mon Sep 17 00:00:00 2001 From: Dylan Trotter Date: Tue, 10 Nov 2026 12:18:39 +0000 Subject: [PATCH 3/4] stewards: address review round 2 (BB-288) Drop the guidance-unit cap: the compact snippets keep a realistically sized index well under the wake cap, and the block is clippable, so a large project still cannot cost the snapshot or "What changed". Add a wake test with the real 75-unit size asserting every id is listed, and keep a pathological-size test for the clipping safety net. --- plugins/stewards/wake.test.ts | 31 +++++++++++++++++++++++-------- plugins/stewards/wake.ts | 6 +----- 2 files changed, 24 insertions(+), 13 deletions(-) diff --git a/plugins/stewards/wake.test.ts b/plugins/stewards/wake.test.ts index c5dd19f8..3472f2c0 100644 --- a/plugins/stewards/wake.test.ts +++ b/plugins/stewards/wake.test.ts @@ -7,7 +7,7 @@ import { comment, issue, pull } from "./fixtures.js"; import { gateState } from "./ready-gate.js"; import { MIGRATIONS, RESOLUTION_KINDS, createStore } from "./store.js"; import { DEPS_PRS, GITHUB_PRS, GITHUB_REVIEWS, LINEAR_TICKETS, OPS_WATCH, DEPS_ALERTS, PLAYBOOK_TEMPLATES } from "./templates.js"; -import { STANDING_RULES, BRIEF_PREFIX, formatBrief, formatDepsWake, formatOpsWake, formatProjectWake, formatRecords, formatWake, GUIDANCE_UNITS_MAX, initiativeProgress, initiativeProgressFlag, spawnCommand, WAKE_MAX_CHARS, workerFinishNote, workerQuestion, type DigestRecords, type WakeInput } from "./wake.js"; +import { STANDING_RULES, BRIEF_PREFIX, formatBrief, formatDepsWake, formatOpsWake, formatProjectWake, formatRecords, formatWake, initiativeProgress, initiativeProgressFlag, spawnCommand, WAKE_MAX_CHARS, workerFinishNote, workerQuestion, type DigestRecords, type WakeInput } from "./wake.js"; import { BRIEF_MAX_CHARS } from "./server.js"; import type { OpsFinding } from "./ops.js"; import type { DepsFinding } from "./deps.js"; @@ -401,20 +401,35 @@ describe("the nightly prompt", () => { expect(formatWake(wakeInput())).not.toContain("## Guidance units"); }); - it("keeps the snapshot and What changed when the guidance index is large (BB-288)", () => { - // A realistically sized AGENTS.md: far more units than a wake lists, each - // with a long paragraph. The compact, capped, clippable block must not - // push the wake past its cap or cost the snapshot or the changes. - const units = Array.from({ length: 300 }, (_, i) => ({ + it("lists every guidance unit in a realistically sized wake, keeping the snapshot and What changed (BB-288)", () => { + // This project's real `.bb`: 52 AGENTS.md units and 23 playbook units. + const units = Array.from({ length: 75 }, (_, i) => ({ id: `gu_${String(i).padStart(16, "0")}`, - file: "AGENTS.md", + file: i < 52 ? "AGENTS.md" : "stewards/playbooks/linear-tickets.md", heading: `Section ${i}`, text: `Guidance paragraph number ${i}. `.repeat(40), lessonId: null, })); const wake = formatWake(wakeInput({ guidance: units, changes: ["Something changed."] })); expect(wake).toContain("## Guidance units"); - expect(wake).toContain(`and ${units.length - GUIDANCE_UNITS_MAX} more`); + for (const u of units) expect(wake).toContain(`- ${u.id} `); + expect(wake).toContain("## 4. Snapshot"); + expect(wake).toContain("## 5. What changed since your last turn"); + expect(wake).toContain("- Something changed."); + expect(wake.length).toBeLessThanOrEqual(WAKE_MAX_CHARS + 200); + }); + + it("keeps the snapshot and What changed when the guidance index is pathologically large (BB-288)", () => { + // More units than any wake could show: the compact block is clipped in the + // snapshot, never the snapshot or the changes. + const units = Array.from({ length: 1_000 }, (_, i) => ({ + id: `gu_${String(i).padStart(16, "0")}`, + file: "AGENTS.md", + heading: `Section ${i}`, + text: `Guidance paragraph number ${i}. `.repeat(40), + lessonId: null, + })); + const wake = formatWake(wakeInput({ guidance: units, changes: ["Something changed."] })); expect(wake).toContain("## 4. Snapshot"); expect(wake).toContain("## 5. What changed since your last turn"); expect(wake).toContain("- Something changed."); diff --git a/plugins/stewards/wake.ts b/plugins/stewards/wake.ts index 88ead369..c67a6cd7 100644 --- a/plugins/stewards/wake.ts +++ b/plugins/stewards/wake.ts @@ -25,8 +25,6 @@ import { needChoices, ESCALATED_NEED_KINDS, RESOLUTION_KINDS, STEWARD_NEED_KINDS /** A wake longer than this is cut in its snapshot, never in what changed or what to do. */ export const WAKE_MAX_CHARS = 30_000; -/** How many guidance units a wake or brief lists before it says how many it left out (BB-288). */ -export const GUIDANCE_UNITS_MAX = 60; /** The characters of a unit's own text a wake or brief shows; the `gu_` id is what is cited (BB-288). */ const GUIDANCE_UNIT_CHARS = 120; @@ -1239,14 +1237,12 @@ function trialLessonsLines(lessons: Lesson[]): string[] { */ function guidanceUnitLines(units: readonly GuidanceUnit[]): string[] { if (units.length === 0) return []; - const shown = units.slice(0, GUIDANCE_UNITS_MAX); return [ "", "## Guidance units", "", "Each hand-written paragraph of `AGENTS.md` and the playbooks carries its `gu_` id. Cite the id of the paragraph a decision or turn rested on in `steward_update { resolve, cites: [...] }` (or `{ forward, cites }`) or `steward_report { cites: [...] }`.", - ...shown.map((u) => `- ${u.id} (${u.file}${u.heading === null ? "" : ` §${u.heading}`}): ${oneLine(u.text, GUIDANCE_UNIT_CHARS)}`), - ...(units.length > shown.length ? [`- … and ${units.length - shown.length} more; run \`bb stewards lessons --efficacy\` for the rest.`] : []), + ...units.map((u) => `- ${u.id} (${u.file}${u.heading === null ? "" : ` §${u.heading}`}): ${oneLine(u.text, GUIDANCE_UNIT_CHARS)}`), ]; } From 362f4063a856e8b71c89ff3531e85d05cc2e0b75 Mon Sep 17 00:00:00 2001 From: Dylan Trotter Date: Tue, 10 Nov 2026 12:26:03 +0000 Subject: [PATCH 4/4] stewards: drop the lesson approval marker (BB-288) The project steward declined the new recordProjectDecision call in mergeLessonPr: posting a project-steward bb-approval marker on a lesson pull request is new code that records a project-steward approval, which the user's decision reserves. A playbook lesson now merges through mergeLessonPr as it did before, with no marker; the project steward's own `complete` is the decision. The e2e asserts no bb-approval marker is posted. --- plugins/stewards/server.test.ts | 38 ++++++++++++++++++++------------- plugins/stewards/server.ts | 24 ++++++++------------- 2 files changed, 32 insertions(+), 30 deletions(-) diff --git a/plugins/stewards/server.test.ts b/plugins/stewards/server.test.ts index ad9265d3..ebc24cae 100644 --- a/plugins/stewards/server.test.ts +++ b/plugins/stewards/server.test.ts @@ -122,8 +122,6 @@ function world() { const userTurns: Array<{ threadId: string; text: string; at?: number }> = []; /** What the page's GitHub actions did. */ const githubComments: Array<{ repo: string; number: number; body: string }> = []; - /** The order of the github writer's comment/merge calls, so a test can assert a marker preceded a merge (BB-288). */ - const githubCallOrder: string[] = []; const githubReviews: Array<{ repo: string; number: number; event: string; body: string }> = []; const githubMerges: Array<{ repo: string; number: number; title: string; body: string }> = []; /** Workflow dispatches the fake github serves, in order (BB-216). */ @@ -154,7 +152,6 @@ function world() { const github = { comment: async (repo: string, number: number, body: string) => { githubComments.push({ repo, number, body }); - githubCallOrder.push("comment"); return `IC_${githubComments.length}`; }, review: async (repo: string, number: number, event: "APPROVE" | "REQUEST_CHANGES" | "COMMENT", body: string) => { @@ -170,7 +167,6 @@ function world() { throw new Error(`PUT /repos/${repo}/pulls/${number}/merge failed: 409 {"message":"Head branch was modified. Review and try the merge again."}`); } githubMerges.push({ repo, number, title, body }); - githubCallOrder.push("merge"); // The real merge changes the pull request's state; the fake must too, or // the plugin's own gate-met merge (BB-257) and the reconcile it // schedules would merge the same open snapshot again in a loop. @@ -567,7 +563,7 @@ function world() { }, }; let now = Date.parse("2026-10-14T15:00:00Z"); - return { files, writes, writtenContent, fileContents, repos, primary, entities, projectList, dependabotAlerts, dependabotGraphEmpty, sourceDeployments, sourceFiles, sourceCommits, sourceRuns, sourceCompares, labelEnsureCalls, labelsEnsured, labelEnsureFailures, setLabelEnsureUnavailable: (value: boolean) => { labelEnsureUnavailable = value; }, setPushUnavailable: (value: boolean) => { pushUnavailable = value; }, setLabelEnsureThrow: (value: string | null) => { labelEnsureThrow = value; }, setLayout: (value: "primary" | "directory") => { layout = value; primary = value === "directory" ? null : { dir: "bb", url: "https://github.com/acme/bb.git" }; }, viewers, setLinearMine: (teams: Array<{ unit: string; name: string }>) => { linearMine = teams; }, setMissingStates: (source: "linear" | "github", unit: string, states: string[]) => { missingStates.set(`${source}:${unit.toLowerCase()}`, states); }, setMissingLabel: (source: "linear" | "github", unit: string, labels: string[]) => { missingLabels.set(`${source}:${unit.toLowerCase()}`, labels); }, rpcCalls, githubTrees, githubBranchProtection, githubLabels, setSnapshotError: (id: string, fail: boolean) => { if (fail) snapshotErrors.add(id); else snapshotErrors.delete(id); }, setSearchUnsupported: (value: boolean) => { searchUnsupported = value; }, setSearchError: (value: string | null) => { searchError = value; }, setCreatePullError: (value: string | null) => { createPullError = value; }, setMergePullError: (value: string | null) => { mergePullError = value; }, setMoveHeadOnMerge: (value: boolean) => { moveHeadOnMerge = value; }, setPullDetailsError: (value: string | null) => { pullDetailsError = value; }, setRetryConflict: (value: boolean) => { retryConflict = value; }, setRetryQueued: (value: boolean) => { retryQueued = value; }, threads, deletes, sections, usage, metadata, spawns, sends, retries, retryEvents, queuedDeletes, userTurns, githubComments, githubReviews, githubMerges, githubCallOrder, githubPulls, githubClosedPulls, githubDefaultBranches, githubBranches, githubFiles, githubFileContents, githubDispatches, setMergeDispatchError: (value: string | null) => { mergeDispatchError = value; }, github, sdk, uiPreferences, uiPreferencesWrites, now: () => now, advance: (ms: number) => (now += ms) }; + return { files, writes, writtenContent, fileContents, repos, primary, entities, projectList, dependabotAlerts, dependabotGraphEmpty, sourceDeployments, sourceFiles, sourceCommits, sourceRuns, sourceCompares, labelEnsureCalls, labelsEnsured, labelEnsureFailures, setLabelEnsureUnavailable: (value: boolean) => { labelEnsureUnavailable = value; }, setPushUnavailable: (value: boolean) => { pushUnavailable = value; }, setLabelEnsureThrow: (value: string | null) => { labelEnsureThrow = value; }, setLayout: (value: "primary" | "directory") => { layout = value; primary = value === "directory" ? null : { dir: "bb", url: "https://github.com/acme/bb.git" }; }, viewers, setLinearMine: (teams: Array<{ unit: string; name: string }>) => { linearMine = teams; }, setMissingStates: (source: "linear" | "github", unit: string, states: string[]) => { missingStates.set(`${source}:${unit.toLowerCase()}`, states); }, setMissingLabel: (source: "linear" | "github", unit: string, labels: string[]) => { missingLabels.set(`${source}:${unit.toLowerCase()}`, labels); }, rpcCalls, githubTrees, githubBranchProtection, githubLabels, setSnapshotError: (id: string, fail: boolean) => { if (fail) snapshotErrors.add(id); else snapshotErrors.delete(id); }, setSearchUnsupported: (value: boolean) => { searchUnsupported = value; }, setSearchError: (value: string | null) => { searchError = value; }, setCreatePullError: (value: string | null) => { createPullError = value; }, setMergePullError: (value: string | null) => { mergePullError = value; }, setMoveHeadOnMerge: (value: boolean) => { moveHeadOnMerge = value; }, setPullDetailsError: (value: string | null) => { pullDetailsError = value; }, setRetryConflict: (value: boolean) => { retryConflict = value; }, setRetryQueued: (value: boolean) => { retryQueued = value; }, threads, deletes, sections, usage, metadata, spawns, sends, retries, retryEvents, queuedDeletes, userTurns, githubComments, githubReviews, githubMerges, githubPulls, githubClosedPulls, githubDefaultBranches, githubBranches, githubFiles, githubFileContents, githubDispatches, setMergeDispatchError: (value: string | null) => { mergeDispatchError = value; }, github, sdk, uiPreferences, uiPreferencesWrites, now: () => now, advance: (ms: number) => (now += ms) }; } async function setup(extra: Partial = {}, settings: Record = {}, seedProvider?: string | null | "fail-list" | "fail-set", seedNavigation?: string | null | "fail-list" | "fail-set") { @@ -10607,9 +10603,6 @@ describe("lessons", () => { const t = await adopted(); const recorded = await t.tool("steward_update", { binding: "ENG-12", lesson: { text: lesson, file: LESSON_PLAYBOOK_FILE, scope: "procedure" } }, t.project); const id = /(lsn_[\w-]+)/u.exec(recorded.text)![1]!; - // The lesson's pull request is observable, so the ordinary project - // approval is recorded before the merge (BB-288). - t.entities.set("acme/bb#1", pull({ id: "acme/bb#1", key: "acme/bb#1", url: "https://github.com/acme/bb/pull/1" }, { headSha: "a".repeat(40), base: "main" })); const done = await t.tool("steward_update", { lessonAction: { id, action: "complete" } }, t.project); expect(done).toMatchObject({ error: false, text: expect.stringMatching(new RegExp(`${id}: merged at c{8}`, "u")) }); // Exactly one PR, on the primary repo, against its default branch, from the @@ -10619,13 +10612,10 @@ describe("lessons", () => { expect(pr).toMatchObject({ repo: "acme/bb", head: `guidance-${t.project}`, base: "main", title: expect.stringContaining(id) }); expect(pr.body).toContain(withLessonAnchor(lesson, id)); expect(pr.body).toContain(`_— from bb thread ${t.project}_`); - // The ordinary project approval names the BB-280 playbook entry and is - // posted before the merge. - const marker = t.githubComments.find((c) => c.body.includes("bb-approval")); - expect(marker?.body).toContain('entry="What every steward is told (the playbooks and the project\'s skills)"'); - const mergeAt = t.githubCallOrder.indexOf("merge"); - expect(mergeAt).toBeGreaterThan(-1); - expect(t.githubCallOrder.slice(0, mergeAt)).toContain("comment"); + // The project steward's own `complete` is the approval. No `bb-approval` + // marker is posted on a lesson pull request: recording a project-steward + // approval is the user's code, so this PR adds none (BB-288). + expect(t.githubComments.filter((c) => c.body.includes("bb-approval"))).toEqual([]); // No person is asked: the project steward squash-merges it. expect(t.githubMerges).toHaveLength(1); expect(t.hooks.store.lesson(id)).toMatchObject({ status: "merged", tier: "promoted", mergeSha: "c".repeat(40), branch: `refs/heads/guidance-${t.project}`, pr: pr.url }); @@ -11346,6 +11336,24 @@ describe("guidance units and the efficacy view (BB-35 step 6)", () => { expect(t.hooks.store.guidanceCitations("p1")).toEqual([{ unitId, lastCitedAt: expect.any(Number) }]); }); + it("lists the binding's playbook units before AGENTS.md, so a clipped wake keeps the playbook ids (BB-288)", async () => { + const t = await adopted(); + const agentsText = "An AGENTS.md rule."; + const playbookText = "A playbook rule added for the test."; + t.files["AGENTS.md"] = `# Guidance\n\n${agentsText}\n`; + t.files[playbookPath("linear-tickets")] = LINEAR_TICKETS.replace("You steward one Linear ticket", `${playbookText}\n\nYou steward one Linear ticket`); + t.advance(6 * 60_000); + t.entities.set("uuid-12", issue({ state: "In Progress", stateType: "started" })); + t.sends.length = 0; + await t.ring("linear", "uuid-12"); + const wake = t.sends.filter((s) => s.threadId === t.steward).at(-1)!.text; + const playbookAt = wake.indexOf(guidanceUnitId(playbookText)); + const agentsAt = wake.indexOf(guidanceUnitId(agentsText)); + expect(playbookAt).toBeGreaterThan(-1); + expect(agentsAt).toBeGreaterThan(-1); + expect(playbookAt).toBeLessThan(agentsAt); + }); + it("prints the escalation trend and the uncited guidance units", async () => { const t = await adopted(); const cited = "Cited recently."; diff --git a/plugins/stewards/server.ts b/plugins/stewards/server.ts index f9367ea8..1e5963b6 100644 --- a/plugins/stewards/server.ts +++ b/plugins/stewards/server.ts @@ -97,7 +97,7 @@ import { classify, hasFingerprintLine, lookChanges, opsTargetKey, parseOpsTarget import { EVENTS_PER_LOOK, NO_TOKEN, RECENT_PER_LOOK, SENTRY_API_URL, TOKEN_SCOPES, createSentryReader, emptySentryLook, issueInOrg, recentLine, sentryChanges, sentryResolveTargets, sentrySightings, sentrySummary, type SentryFinding, type SentryLook, type SentryReader } from "./sentry.js"; import { advanced, answerDiffers, anyWorkerInFlight, asksForOpenApproval, commentDiffersFromRecommendation, compactDue, deploymentSettled, diffObserved, endingDeadline, followingThrough, gateKnownUnmet, hasLabel, IDLE_WAKE_TEXT, idleInActiveStage, isCommentNews, isOwnStageChange, lifecycle, mechanicalStatus, namesUserAsBlocker, newHumanComments, newestCommentAt, openUserFacingItems, ownChecksFinished, ownPullClosed, ownPullMerged, phaseAndStage, productGateOptions, runningWorkerReason, newestEditComment, specDiff, specHash, specText, stuckDue, unheardWait, workerInFlight, workerReportedTerminal, type Change, type MechanicalEvent, type OpenUserItem } from "./reconcile.js"; import { classifyApproval, latestProjectApprovalMarker, latestReviewMarker, prGate, reviewMarker } from "./gate.js"; -import { approveOwnerReviewedPull, approvePrPageAction, declineOwnerReviewedPull, fileForwardedApprovePrNeed, isProjectSteward, isUserThread, markerKey, normalizeText, placeReportedPull, recordProjectDecision, specApprovedByProject, syncPullApproval, userApprovedOnEntity, userSaid, type ApprovalsContext } from "./approvals.js"; +import { approveOwnerReviewedPull, approvePrPageAction, declineOwnerReviewedPull, fileForwardedApprovePrNeed, isProjectSteward, isUserThread, markerKey, normalizeText, placeReportedPull, specApprovedByProject, syncPullApproval, userApprovedOnEntity, userSaid, type ApprovalsContext } from "./approvals.js"; import { deliverEscalation, nextRoute, routedTo, type EscalationsContext } from "./escalations.js"; import { approvalWanted, briefGate, gateState, resolveChecklistItem, syncApprovalRequest, type ReadyGateContext } from "./ready-gate.js"; import { @@ -1961,10 +1961,12 @@ export function createStewardsPlugin(deps: StewardsDeps): (bb: BbPluginApi) => P store.setServerSetting(guidanceRenderedKey(projectId), String(now())); } - /** The AGENTS.md and playbook units a wake renders: `AGENTS.md` always, plus the binding's playbook (BB-288). */ + /** The AGENTS.md and playbook units a wake renders: the binding's playbook first, then `AGENTS.md` (BB-288). The order matters because the index is compact but `fit` clips from the end: a clipped wake keeps the playbook ids that bind this binding. */ function guidanceFor(units: readonly GuidanceUnit[], playbookName: string | null): GuidanceUnit[] { const file = playbookName === null ? null : playbookPath(playbookName); - return units.filter((u) => u.file === "AGENTS.md" || (file !== null && u.file === file)); + const playbook = file === null ? [] : units.filter((u) => u.file === file); + const agents = units.filter((u) => u.file === "AGENTS.md"); + return [...playbook, ...agents]; } /** The guidance units for a wake, read (and cached) from `.bb` HEAD, recording the first render (BB-288). */ @@ -6003,8 +6005,10 @@ export function createStewardsPlugin(deps: StewardsDeps): (bb: BbPluginApi) => P // BB-288: the decide list decides who lands the lesson, exactly as it // decides any other pull request. A `user` file (AGENTS.md, the project // file, roles.json) files the owner's Review PR/Drop item; a `project` - // file (a playbook) is the project steward's, and the ordinary project - // approval is recorded before the merge. + // file (a playbook) is the project steward's, whose `complete` is the + // approval. No approval marker is posted: recording a project-steward + // approval is the user's code, and the project steward's own `complete` + // already carries the decision. const decisions = (await loadConfig(lesson.projectId))?.file.bigDecisions ?? []; if (lessonLandingMark(lesson, decisions) === "user") { const filed = fileLessonOwnerNeed(lesson, result.branch, result.url, retire); @@ -6014,16 +6018,6 @@ export function createStewardsPlugin(deps: StewardsDeps): (bb: BbPluginApi) => P store.noteLesson(lesson.id, message); return { ok: false, message }; } - const binding = lesson.bindingId === null ? null : store.binding(lesson.bindingId); - const projectThreadId = store.project(lesson.projectId).threadId; - const pull = await snapshot("github", `${result.repo}#${result.number}`).catch(() => null); - if (binding !== null && projectThreadId !== null && pull !== null && (pull.pull?.headSha ?? "") !== "") { - const files = github.files === undefined ? null : await github.files(result.repo, result.number).catch(() => null); - const entry = files === null ? null : (explainDecision(files, decisions).winner?.text ?? null); - await recordProjectDecision(approvalsCtx, binding, pull, projectThreadId, { headSha: pull.pull?.headSha ?? "", entry, verdict: "approved", reasoning: note }).catch((error: unknown) => { - bb.log.warn(`stewards: could not record the project approval for lesson ${lesson.id}: ${errorMessage(error)}`); - }); - } try { const { sha } = await github.squashMerge(result.repo, result.number, `Lesson ${lesson.id}`, note); store.markLessonMerged(lesson.id, sha, result.branch ?? lesson.branch, note);