diff --git a/plugins/stewards/DESIGN.md b/plugins/stewards/DESIGN.md index dfedd75..b7d78ae 100644 --- a/plugins/stewards/DESIGN.md +++ b/plugins/stewards/DESIGN.md @@ -981,7 +981,10 @@ deployment: # where a merge is confirmed live (BB-53); omi The cross-plugin RPC contract in §3, plus `mergeGuidance`: the narrow fast-forward of the canonical `.bb` to a thread's published guidance branch -that a self-merged lesson uses (§20). Only the stewards plugin may call it. +for a standalone `.bb` repo. Since BB-108 a lesson lands as a pull request on +the primary repo and stewards no longer calls `mergeGuidance`; `pushGuidance` +(`workspace_publish_guidance`) is still the branch push. Only the stewards +plugin may call `mergeGuidance`. ### bb-gate @@ -1498,52 +1501,59 @@ A **lesson** makes the answer a record and, for most of them, lands it. cluster). A much-later forward of the same wording under the same playbook therefore gets no page box for a why the user already gave in the project steward's thread — the suggestion step 4 deferred. -- **The promotion and retirement write only the lesson's own anchored text.** - A promotion the nightly pass queues is listed under `## Lessons to land`; - the project steward adds the block (`` and the wording) - to the named file, commits, publishes with `workspace_publish_guidance`, and - completes it. A retirement is listed under `## Lessons to retire`; the - project steward replaces the block with `` and - publishes, then `steward_update { lessonAction: { action: "retire" } }` - fast-forwards the canonical checkout. The same multi-repo guard applies both - ways: only the lesson's own file may change, and it must carry the anchor the - call names. A lesson's `file` may never be `stewards/project.json`, so a - lesson can never move a big decision to the project, loosen the - pull-request gate or relax an approval; the decide list, `bigDecisions` and - `prGate` are outside a lesson's reach by construction. A boundary-scope - lesson still becomes a Merge/Drop item for the user. `retireLessonAnchor` - in `lessons.ts` is the normative definition of the retirement edit; the - project steward makes it by hand and the helper is what the tests pin. -- **Procedure lessons self-merge.** Playbook procedure, sequencing and - conventions are what the project steward may already decide. It applies - the text to the file in its `.bb` checkout, commits, and publishes with - `workspace_publish_guidance`; then - `steward_update { lessonAction: { id, action: "complete" } }` asks - multi-repo to fast-forward the canonical checkout to that branch. The - fast-forward is deliberately narrow: the canonical checkout must be clean - and not detached, the branch must be a fast-forward whose commits change - only the lesson's own file, and that file must carry the lesson's text - (whitespace-normalised). Anything else is refused and the - `guidance-` branch is left where it is. The merge commit's sha is - recorded on the lesson. This is the only way a self-merged lesson reaches - the HEAD that `readProjectFiles` — and so every steward — reads. -- **Boundary lessons go to the user.** Anything that would change the - steward's own escalation boundary — credentials and reach, who may direct - a server, irreversible production steps — is never self-merged. The - project steward applies, commits and publishes it, then `complete` files a - BB-15 `decide` item on the binding with the fixed choices **Merge** and - **Drop**. Merge runs the same multi-repo fast-forward from the page, as - the connected user and unsigned; Drop marks the lesson dropped. Both close - the item and wake the steward. -- **The guard.** multi-repo exposes the merge as an RPC that refuses any - caller but the stewards plugin, and stewards only calls it for a pending - lesson the project steward recorded. The fast-forward can therefore only - change the lesson's own file, and only to a version carrying the lesson - text; other edits already in that file ride along, which the recorded sha - and the revert below cover. +- **The promotion and retirement write only the lesson's own anchored text, + as a pull request (BB-108).** A promotion the nightly pass queues is listed + under `## Lessons to land`; the project steward adds the block + (`` and the wording) to the named file, publishes with + `workspace_publish_guidance`, and completes it. A retirement is listed under + `## Lessons to retire`; the project steward replaces the block with + ``, publishes, then + `steward_update { lessonAction: { action: "retire" } }`. With `.bb` a + directory on the primary repo's default branch (BB-106) there is nothing to + fast-forward, so the plugin opens a pull request on the primary repo's + origin from the published `guidance-` branch: one per lesson, or one + per publish when several lessons touch the same file. The body names the + lesson id, the anchor and the proposing steward, and is signed. The PR path + refuses before anything is opened: a lesson's `file` may never be + `stewards/project.json`, `stewards/roles.json` or `bigDecisions`, so a + lesson can never move a big decision to the project, loosen the pull-request + gate, relax an approval or change who is a steward; the decide list, + `bigDecisions`, `roles.json` and `prGate` are outside a lesson's reach by + construction. `retireLessonAnchor` in `lessons.ts` is the normative + definition of the retirement edit; the project steward makes it by hand and + the helper is what the tests pin. +- **Procedure lessons are merged by the project steward.** Playbook procedure, + sequencing and conventions are what the project steward may already decide. + It applies the text to the file in its `.bb` checkout, publishes with + `workspace_publish_guidance`, and `complete` opens the pull request; the + project steward squash-merges it at once through the server's gh login, with + no person asked, and the merge commit's sha is recorded on the lesson. This + is the only way a self-merged lesson reaches the HEAD that + `readProjectFiles` — and so every steward — reads. +- **Boundary lessons wait for the owner's approval.** Anything that would + change the steward's own escalation boundary — credentials and reach, who + may direct a server, irreversible production steps — is never merged by the + plugin. The project steward publishes it, `complete` opens the pull request + and files a BB-15 `decide` item on the binding with the fixed choices + **Review PR** and **Drop**. Review PR points the owner at the pull request; + the owner approves and merges it on GitHub, and a project-steward approval + never counts for it. The lesson is marked merged when the pull request + merges. Drop closes the pull request, marks the lesson dropped and closes + the item. +- **The guard.** The self-widening guard is enforced in the PR path before a + branch is pushed or a pull request is opened, so a lesson row that predates + the record-time guard can never move the decide list or the roles split. + Before a procedure merge the plugin re-checks the pull request: every changed + path must be a file of a lesson published on that branch, and this lesson's + file must carry the anchor the call names. Anything else is refused and noted + on the lesson, so a lesson can never smuggle a second change into an + auto-merged pull request. A reviewer still sees the diff like any other + change. - **Status is observed, not assumed.** Every sweep and nightly pass reads each pending lesson's file at `.bb` HEAD and marks it `merged` when the - anchored text is there, however it got there; a retired lesson is skipped so + anchored text is there, however it got there, and reads a pending lesson's + pull request through the github source and marks it `merged` when it + merged, closing the owner's Review PR item; a retired lesson is skipped so an observation never resurrects it. `dropped` is set by the user's Drop button, the project steward's `drop` action, a retirement, or `bb stewards drop-lesson `. @@ -1560,13 +1570,13 @@ A **lesson** makes the answer a record and, for most of them, lands it. `bb stewards lessons [--all]` lists them all with their counts, and `bb stewards lessons --efficacy` prints the escalation-to-user trend and the uncited guidance units; `bb stewards drop-lesson ` drops one. -- **Undo.** A merged procedure lesson is an ordinary commit on `.bb` HEAD; - `bb stewards lessons --all` names its `mergeSha` and branch. To undo it, - revert that commit in the canonical checkout (`git revert ` in the - project's `.bb`, then push), or let the nightly pass retire it once it is - uncited for `promoted.retireDays` and remove the anchored block through - `## Lessons to retire`. The row is kept with `tier: retired` either way. A - boundary lesson is undone the same way once the user has merged it. A +- **Undo.** A merged lesson is an ordinary commit on the primary repo's + default branch; `bb stewards lessons --all` names its `mergeSha` and branch. + To undo it, open a revert pull request on the primary repo, or let the + nightly pass retire it once it is uncited for `promoted.retireDays` and + remove the anchored block through `## Lessons to retire` (itself a pull + request). The row is kept with `tier: retired` either way. A + boundary lesson is undone the same way once the owner has merged it. A lesson recorded by an image before this step gets the column default `candidate`, and the remap has already run at its own index, so after a roll-forward it is invisible to wakes; `bb stewards lessons --all` still diff --git a/plugins/stewards/README.md b/plugins/stewards/README.md index 0bc06cb..88d0074 100644 --- a/plugins/stewards/README.md +++ b/plugins/stewards/README.md @@ -308,20 +308,26 @@ bb. [DESIGN.md](DESIGN.md) is the design; this is how it is built and run. one at 90 days uncited or 2 corrections). The thresholds, caps and decay half-life are under `lessons` in `stewards/project.json`; the design's defaults apply when the key is absent. Only a `procedure` lesson is promoted - mechanically; a `boundary` one stays `trial` until you Merge it, and two - rare validation escalations are capped at `lessons.validations.perWeek` - (default 1) per week, ordered by score: the boundary Merge/Drop, and one - Keep/Retire question when a promoted lesson has citations and a correction - short of the retire count. Silence leaves a boundary lesson in trial and - retires a mixed-evidence one on the next nightly pass; a Keep is remembered. + mechanically; a `boundary` one stays `trial` until its pull request merges, + and two rare validation escalations are capped at + `lessons.validations.perWeek` (default 1) per week, ordered by score: the + boundary Review PR/Drop, and one Keep/Retire question when a promoted lesson + has citations and a correction short of the retire count. Silence leaves a + boundary lesson in trial and retires a mixed-evidence one on the next nightly + pass; a Keep is remembered. Trial lessons are injected into - wakes under `## Lessons (trial)`; only promoted ones reach `.bb`, with a - `` anchor, and only that lesson's own file (multi-repo - fast-forwards the canonical `.bb`). A `boundary` lesson — credentials and - reach, who may direct a server, irreversible production steps — is filed on - the Stewards page with **Merge** and **Drop**, and only you merge it; a - lesson can never target `stewards/project.json`, so it cannot move a big - decision to the project, loosen the pull-request gate or relax an approval. + wakes under `## Lessons (trial)`; only promoted ones reach the primary + repo, with a `` anchor, and only that lesson's own file. + Because `.bb` is a directory on the primary repo's default branch (BB-106), + a lesson lands as a pull request, never a fast-forward: the plugin opens one + PR per lesson (or per publish when several touch the same file). A + `procedure` lesson's PR is merged by the project steward at once; a + `boundary` lesson — credentials and reach, who may direct a server, + irreversible production steps — is filed on the Stewards page with + **Review PR** and **Drop**, and only you merge the PR (a project-steward + approval never counts). A lesson can never target `stewards/project.json` or + `stewards/roles.json`, so it cannot move a big decision to the project, + loosen the pull-request gate, relax an approval or change who is a steward. A user's answer to a forwarded escalation on the Stewards page (or from a thread) wakes the project steward with that answer as a lesson candidate: if it is a general judgement the playbook should carry, the project steward diff --git a/plugins/stewards/app.tsx b/plugins/stewards/app.tsx index 24e2b19..42547af 100644 --- a/plugins/stewards/app.tsx +++ b/plugins/stewards/app.tsx @@ -1099,7 +1099,7 @@ function NeedControls({ need: n, onDone }: { need: BoardNeed; onDone: () => void const freeText = choices.length === 0 || other; // The entity link opens the binding's ticket; for approve-pr and merge the // item's `url` is the pull request, so it gets its own link. - const prUrl = n.kind === "approve-pr" || n.kind === "merge" ? n.url : null; + const prUrl = n.kind === "approve-pr" || n.kind === "merge" ? n.url : (n.prUrl ?? null); return ( <> {takesNote && !lesson ? ( @@ -1167,6 +1167,7 @@ function NeedControls({ need: n, onDone }: { need: BoardNeed; onDone: () => void Open the pull request ) : null} + {lesson && n.prUrl === null && n.kind === "decide" && n.choices.includes("Review PR") ? The pull request is not open yet. : null} {typeof state === "object" ? {state.error} : null} diff --git a/plugins/stewards/github.ts b/plugins/stewards/github.ts index 64a4fcf..e024c93 100644 --- a/plugins/stewards/github.ts +++ b/plugins/stewards/github.ts @@ -53,14 +53,26 @@ export interface GithubWriter { * hand-typed lookalike never satisfies the lane gate. */ comment(repo: string, number: number, body: string): Promise; - /** Squash-merge the pull request, with the given commit title and message. */ - squashMerge(repo: string, number: number, title: string, body: string): Promise; + /** Squash-merge the pull request, with the given commit title and message; returns the merge commit sha when GitHub reports one. */ + squashMerge(repo: string, number: number, title: string, body: string): Promise<{ sha: string | null }>; /** * The repo-relative paths a pull request changes, for the decide list. When * absent, or when the read fails, the plugin cannot classify the pull * request and treats it as the user's (fail closed). */ files?(repo: string, number: number): Promise; + /** A repository's default branch, for the base of a lesson's pull request (BB-108). */ + defaultBranch(repo: string): Promise; + /** Open a pull request and return its URL and number (BB-108). */ + createPull(repo: string, args: { head: string; base: string; title: string; body: string }): Promise<{ url: string; number: number }>; + /** Close a pull request without merging it (BB-108 Drop). */ + closePull(repo: string, number: number): Promise; + /** + * One file's UTF-8 content at a ref, or null when it is not there. Used to + * check a lesson's anchored block is really in the pull request before a + * procedure merge (BB-108). + */ + fileContent?(repo: string, path: string, ref: string): Promise; } /** "owner/repo#12" or "https://github.com/owner/repo/pull/12" → { repo, number }. */ @@ -100,7 +112,10 @@ export function createGhWriter(token: TokenReader = createGhTokenReader(), doFet return String(nodeId); }, async squashMerge(repo, number, title, body) { - await request("PUT", `/repos/${repo}/pulls/${number}/merge`, { merge_method: "squash", commit_title: title, commit_message: body }); + const response = await request("PUT", `/repos/${repo}/pulls/${number}/merge`, { merge_method: "squash", commit_title: title, commit_message: body }); + const merged = (await response.json().catch(() => null)) as { sha?: unknown } | null; + const sha = merged?.sha; + return { sha: typeof sha === "string" && sha !== "" ? sha : null }; }, async files(repo, number) { const response = await request("GET", `/repos/${repo}/pulls/${number}/files?per_page=100`, undefined); @@ -108,5 +123,30 @@ export function createGhWriter(token: TokenReader = createGhTokenReader(), doFet if (!Array.isArray(body)) return []; return body.map((f) => (f !== null && typeof f === "object" && "filename" in f ? String((f as { filename: unknown }).filename) : "")).filter((f) => f !== ""); }, + async defaultBranch(repo) { + const response = await request("GET", `/repos/${repo}`, undefined); + const body = (await response.json().catch(() => null)) as { default_branch?: unknown } | null; + return typeof body?.default_branch === "string" && body.default_branch !== "" ? body.default_branch : null; + }, + async createPull(repo, args) { + const response = await request("POST", `/repos/${repo}/pulls`, { head: args.head, base: args.base, title: args.title, body: args.body }); + const body = (await response.json().catch(() => null)) as { html_url?: unknown; number?: unknown } | null; + if (typeof body?.html_url !== "string" || typeof body.number !== "number") throw new GithubApiError("GitHub created the pull request but returned no URL or number."); + return { url: body.html_url, number: body.number }; + }, + async closePull(repo, number) { + await request("PATCH", `/repos/${repo}/pulls/${number}`, { state: "closed" }); + }, + async fileContent(repo, path, ref) { + try { + const response = await request("GET", `/repos/${repo}/contents/${path.split("/").map(encodeURIComponent).join("/")}?ref=${encodeURIComponent(ref)}`, undefined); + const body = (await response.json().catch(() => null)) as { content?: unknown; encoding?: unknown } | null; + if (body === null || typeof body.content !== "string") return null; + return body.encoding === "base64" ? Buffer.from(body.content, "base64").toString("utf8") : body.content; + } catch (error) { + if (error instanceof GithubApiError && / 404 /u.test(error.message)) return null; + throw error; + } + }, }; } diff --git a/plugins/stewards/server.test.ts b/plugins/stewards/server.test.ts index 9c7bf32..e2e9887 100644 --- a/plugins/stewards/server.test.ts +++ b/plugins/stewards/server.test.ts @@ -25,6 +25,9 @@ function world() { [playbookPath("github-reviews")]: GITHUB_REVIEWS, }; const repos = [{ dir: "web", url: "https://github.com/acme/web.git" }]; + /** The primary repo a lesson's pull request opens on (BB-108). */ + const primary = { dir: "bb", url: "https://github.com/acme/bb.git" }; + const layout: "primary" | "standalone" = "primary"; const entities = new Map(); /** Dependabot alerts the fake github serves, by lowercased repo. */ const dependabotAlerts = new Map(); @@ -33,9 +36,6 @@ function world() { /** Deployments the fake github serves, by `lowercased repo lowercased environment`. */ const sourceDeployments = new Map(); const rpcCalls: Call[] = []; - /** Every lesson merge the plugin asked multi-repo for, and what it answered. */ - const mergeGuidanceCalls: Record[] = []; - let mergeGuidanceResult: { ok: boolean; sha: string | null; branch: string | null; message: string } | null = null; const threads = new Map(); /** Every `threads.delete` the plugin asked for; the plugin must only ever archive. */ const deletes: string[] = []; @@ -57,17 +57,55 @@ function world() { /** What the page's GitHub actions did. */ const githubComments: Array<{ repo: string; number: number; body: string }> = []; const githubMerges: Array<{ repo: string; number: number; title: string; body: string }> = []; + /** Every lesson pull request the plugin opened, and every one it closed (BB-108). */ + const githubPulls: Array<{ repo: string; head: string; base: string; title: string; body: string; url: string; number: number }> = []; + const githubClosedPulls: Array<{ repo: string; number: number }> = []; + const githubDefaultBranches = new Map(); + let pullNumber = 0; + let createPullError: string | null = null; + let mergePullError: string | null = null; /** The changed paths `github.files` reports, by "owner/repo#n". */ const githubFiles = new Map(); + /** Overrides for `github.fileContent`, by "owner/repo#n:path". */ + const githubFileContents = new Map(); const github = { comment: async (repo: string, number: number, body: string) => { githubComments.push({ repo, number, body }); return `IC_${githubComments.length}`; }, squashMerge: async (repo: string, number: number, title: string, body: string) => { + if (mergePullError !== null) throw new Error(mergePullError); githubMerges.push({ repo, number, title, body }); + return { sha: "c".repeat(40) }; + }, + files: async (repo: string, number: number) => { + const explicit = githubFiles.get(`${repo}#${number}`); + if (explicit !== undefined) return explicit; + // A lesson PR's changed file is the one named in its body. + const pr = githubPulls.find((p) => p.repo === repo && p.number === number); + const m = pr === undefined ? null : /in `([^`]+)`/u.exec(pr.body); + return m === null ? [] : [`.bb/${m[1]}`]; + }, + fileContent: async (repo: string, path: string, ref: string) => { + const explicit = githubFileContents.get(`${repo}#${ref}:${path}`); + if (explicit !== undefined) return explicit; + // The branch ref carries the latest publish, so the last PR on it. + const pr = githubPulls.filter((p) => p.repo === repo && (p.head === ref || `refs/heads/${p.head}` === ref)).at(-1); + if (pr === undefined) return null; + const m = /in `([^`]+)`/u.exec(pr.body); + return m === null || path !== `.bb/${m[1]}` ? null : pr.body; + }, + defaultBranch: async (repo: string) => githubDefaultBranches.get(repo) ?? "main", + createPull: async (repo: string, args: { head: string; base: string; title: string; body: string }) => { + if (createPullError !== null) throw new Error(createPullError); + const number = ++pullNumber; + const url = `https://github.com/${repo}/pull/${number}`; + githubPulls.push({ repo, ...args, url, number }); + return { url, number }; + }, + closePull: async (repo: string, number: number) => { + githubClosedPulls.push({ repo, number }); }, - files: async (repo: string, number: number) => githubFiles.get(`${repo}#${number}`) ?? [], }; let n = 0; @@ -80,7 +118,7 @@ function world() { if (paths.some((path) => path.startsWith("/") || path.includes("\\") || path.split("/").includes(".."))) throw new Error(`bad project file path: ${paths.join(", ")}`); out = { projectSourcePath: "/p/.bb", head: "abc", files: paths.map((path) => ({ path, content: files[path] ?? null })), error: null }; } - else if (args.method === "repoSet") out = { repos, error: null }; + else if (args.method === "repoSet") out = { repos, primary, layout, error: null }; else if (args.method === "sourceSnapshot") out = { snapshot: entities.get(String(input.id)) ?? null }; else if (args.method === "sourceSetInterest") out = { ok: true }; else if (args.method === "sourceAdoptable") { @@ -105,10 +143,6 @@ function world() { const key = `${String(input.repo).toLowerCase()} ${String(input.environment).toLowerCase()}`; out = { deployments: sourceDeployments.get(key) ?? [], error: null }; } - else if (args.method === "mergeGuidance") { - mergeGuidanceCalls.push(input); - out = mergeGuidanceResult ?? { ok: true, sha: "b".repeat(40), branch: `refs/heads/guidance-${String(input.threadId)}`, message: "merged" }; - } else throw Object.assign(new Error(`unknown ${args.pluginId}.${args.method}`), { status: 404 }); return args.outputSchema.parse(out); }; @@ -284,7 +318,7 @@ function world() { }, }; let now = Date.parse("2026-10-14T15:00:00Z"); - return { files, writes, fileContents, repos, entities, dependabotAlerts, sourceDeployments, viewers, rpcCalls, mergeGuidanceCalls, setMergeGuidanceResult: (value: typeof mergeGuidanceResult) => { mergeGuidanceResult = value; }, threads, deletes, sections, usage, metadata, spawns, sends, userTurns, githubComments, githubMerges, githubFiles, github, sdk, uiPreferences, uiPreferencesWrites, now: () => now, advance: (ms: number) => (now += ms) }; + return { files, writes, fileContents, repos, primary, entities, dependabotAlerts, sourceDeployments, viewers, rpcCalls, setCreatePullError: (value: string | null) => { createPullError = value; }, setMergePullError: (value: string | null) => { mergePullError = value; }, threads, deletes, sections, usage, metadata, spawns, sends, userTurns, githubComments, githubMerges, githubPulls, githubClosedPulls, githubDefaultBranches, githubFiles, githubFileContents, 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") { @@ -6172,56 +6206,212 @@ describe("lessons", () => { expect(t.hooks.store.lesson(id)).toMatchObject({ escalationId: esc, bindingId: t.hooks.store.find("ENG-12")!.id }); }); - it("a procedure lesson is self-merged through multi-repo and marked merged at the returned sha", async () => { + 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 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")) }); + // Exactly one PR, on the primary repo, against its default branch, from the + // guidance branch, carrying the anchored block and signed. + expect(t.githubPulls).toHaveLength(1); + const pr = t.githubPulls[0]!; + 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}_`); + // 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 }); + }); + + 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 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); + expect(t.hooks.store.lesson(id)).toMatchObject({ status: "pending", note: "No commits between main and guidance-x." }); + }); + + 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 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); + expect(t.hooks.store.lesson(id)).toMatchObject({ status: "pending", pr: expect.stringContaining("acme/bb/pull/"), note: "Pull request is not mergeable: required status check is expected." }); + }); + + 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 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"]); 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 b{8}`, "u")) }); - expect(t.mergeGuidanceCalls).toEqual([{ projectId: "p1", threadId: t.project, file: "AGENTS.md", text: withLessonAnchor(lesson, id) }]); - expect(t.hooks.store.lesson(id)).toMatchObject({ status: "merged", tier: "promoted", mergeSha: "b".repeat(40), branch: `refs/heads/guidance-${t.project}` }); + expect(done.text).toMatch(/pull request refused: .*other than the lesson's own/u); + expect(t.githubMerges).toEqual([]); + expect(t.hooks.store.lesson(id)).toMatchObject({ status: "pending", note: expect.stringContaining("other than the lesson's own") }); }); - it("a refused procedure merge leaves the lesson pending with the reason", async () => { + it("refuses to merge a procedure lesson's pull request whose file lacks the anchor", async () => { const t = await adopted(); - t.setMergeGuidanceResult({ ok: false, sha: null, branch: "refs/heads/guidance-x", message: "The canonical checkout has uncommitted changes." }); 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]!; + t.githubFileContents.set(`acme/bb#guidance-${t.project}:.bb/AGENTS.md`, "# Guidance\n\nNo anchor here.\n"); const done = await t.tool("steward_update", { lessonAction: { id, action: "complete" } }, t.project); - expect(done.text).toMatch(/merge refused: The canonical checkout has uncommitted changes\./u); - expect(t.hooks.store.lesson(id)).toMatchObject({ status: "pending", note: "The canonical checkout has uncommitted changes." }); + expect(done.text).toMatch(/pull request refused: .*does not carry the lesson's anchor/u); + expect(t.githubMerges).toEqual([]); + expect(t.hooks.store.lesson(id)).toMatchObject({ status: "pending" }); }); - it("a boundary lesson is never self-merged; it is filed for the user with Merge/Drop", async () => { + it("a boundary lesson opens a pull request and is filed for the owner with Review PR/Drop", async () => { const t = await adopted(); const recorded = await t.tool("steward_update", { binding: "ENG-12", lesson: { text: "Only the user may grant a credential.", file: "AGENTS.md", scope: "boundary" } }, 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(/boundary lesson; Merge\/Drop filed/u); - // No merge was attempted for a boundary lesson. - expect(t.mergeGuidanceCalls).toEqual([]); + expect(done.text).toMatch(/boundary lesson; Review PR\/Drop filed/u); + // The pull request is open, but the project steward never merges a + // boundary lesson's PR. + expect(t.githubPulls).toHaveLength(1); + expect(t.githubMerges).toEqual([]); const need = t.hooks.store.openNeeds(t.hooks.store.find("ENG-12")!.id).find((n) => n.kind === "decide" && typeof n.data.lessonId === "string")!; - expect(need).toMatchObject({ url: id, filedBy: "project", data: { lessonId: id, choices: ["Merge", "Drop"] } }); - // The page's Merge runs the same multi-repo step. - const merged = await t.harness.callRpc("stewardsNeedAction", { needId: need.id, action: "choose", choice: "Merge" }, client); - expect(merged).toMatchObject({ ok: true }); - expect(t.mergeGuidanceCalls).toHaveLength(1); - expect(t.hooks.store.lesson(id)).toMatchObject({ status: "merged", mergeSha: "b".repeat(40) }); + expect(need).toMatchObject({ url: id, filedBy: "project", data: { lessonId: id, prUrl: t.githubPulls[0]!.url, choices: ["Review PR", "Drop"] } }); + // Review PR points the owner at the pull request and leaves the item open. + const reviewed = await t.harness.callRpc("stewardsNeedAction", { needId: need.id, action: "choose", choice: "Review PR" }, client); + expect(reviewed).toMatchObject({ ok: true, message: expect.stringContaining(t.githubPulls[0]!.url) }); + expect(t.hooks.store.openNeeds(t.hooks.store.find("ENG-12")!.id)).toContainEqual(expect.objectContaining({ id: need.id })); + expect(t.hooks.store.lesson(id)).toMatchObject({ status: "pending" }); }); - it("merges a boundary lesson from a thread with bb stewards done --choice Merge", async () => { + it("marks a boundary lesson merged when its pull request merges, closing the Review PR item", async () => { const t = await adopted(); const recorded = await t.tool("steward_update", { binding: "ENG-12", lesson: { text: "Only the user may grant a credential.", file: "AGENTS.md", scope: "boundary" } }, t.project); const id = /(lsn_[\w-]+)/u.exec(recorded.text)![1]!; await t.tool("steward_update", { lessonAction: { id, action: "complete" } }, t.project); - const need = t.hooks.store.openNeeds(t.hooks.store.find("ENG-12")!.id).find((n) => n.kind === "decide")!; - const done = await t.harness.behavior.runCli(["done", need.id, "--choice", "Merge", "--project", "p1"]); - expect(done.stdout).toMatch(new RegExp(`Merged ${id}`, "u")); - expect(t.mergeGuidanceCalls).toHaveLength(1); + const prUrl = t.githubPulls[0]!.url; + // The owner approves and merges on GitHub; the next pass observes it + // through the github source, never a fast-forward. + t.entities.set("acme/bb#1", pull({ id: "acme/bb#1", key: "acme/bb#1", url: prUrl }, { merged: true })); + await t.hooks.sweep(); + await t.flush(); expect(t.hooks.store.lesson(id)).toMatchObject({ status: "merged" }); + expect(t.hooks.store.openNeeds(t.hooks.store.find("ENG-12")!.id).find((n) => n.kind === "decide" && n.data.lessonId === id)).toBeUndefined(); }); - it("Drop discards a boundary lesson and closes its item", async () => { + it("reuses one pull request when several lessons are landed in the same publish", async () => { + const t = await adopted(); + const a = await t.tool("steward_update", { binding: "ENG-12", lesson: { text: "Rule A.", file: "AGENTS.md", scope: "boundary" } }, t.project); + const b = await t.tool("steward_update", { binding: "ENG-12", lesson: { text: "Rule B.", file: "AGENTS.md", scope: "boundary" } }, t.project); + const idA = /(lsn_[\w-]+)/u.exec(a.text)![1]!; + const idB = /(lsn_[\w-]+)/u.exec(b.text)![1]!; + await t.tool("steward_update", { lessonAction: { id: idA, action: "complete" } }, t.project); + await t.tool("steward_update", { lessonAction: { id: idB, action: "complete" } }, t.project); + // One branch, one publish, one pull request; each lesson keeps its item. + expect(t.githubPulls).toHaveLength(1); + expect(t.hooks.store.lesson(idA)!.pr).toBe(t.githubPulls[0]!.url); + expect(t.hooks.store.lesson(idB)!.pr).toBe(t.githubPulls[0]!.url); + expect(t.hooks.store.openNeeds(t.hooks.store.find("ENG-12")!.id).filter((n) => n.data.lessonId === idA || n.data.lessonId === idB)).toHaveLength(2); + }); + + it("a boundary retire opens a fresh pull request and is observed when it merges", async () => { + const t = await adopted(); + const binding = t.hooks.store.find("ENG-12")!; + const recorded = await t.tool("steward_update", { binding: "ENG-12", lesson: { text: "Only the user may grant a credential.", file: "AGENTS.md", scope: "boundary" } }, t.project); + const id = /(lsn_[\w-]+)/u.exec(recorded.text)![1]!; + // The lesson was promoted and its promotion PR was merged by the owner. + t.hooks.store.markLessonMerged(id, "d".repeat(40), `refs/heads/guidance-${t.project}`); + t.hooks.store.setLessonPr(id, "https://github.com/acme/bb/pull/7"); + t.hooks.store.setLessonTier(id, "retired"); + const done = await t.tool("steward_update", { lessonAction: { id, action: "retire" } }, t.project); + expect(done.text).toMatch(/boundary retire; Review PR\/Drop filed/u); + // A fresh retire PR, not the merged promotion PR. + const retirePr = t.githubPulls.at(-1)!; + expect(retirePr.body).toContain(retiredLessonAnchor(id)); + expect(t.hooks.store.lesson(id)).toMatchObject({ tier: "retired", status: "merged", retirePr: retirePr.url }); + const need = t.hooks.store.openNeeds(binding.id).find((n) => n.kind === "decide" && n.data.lessonId === id)!; + expect(need).toMatchObject({ data: { retire: true, prUrl: retirePr.url, choices: ["Review PR", "Drop"] } }); + // The owner merges the retire PR: the row is dropped and the item closes. + const number = /pull\/(\d+)/u.exec(retirePr.url)![1]!; + t.entities.set(`acme/bb#${number}`, pull({ id: `acme/bb#${number}`, key: `acme/bb#${number}` }, { merged: true })); + await t.hooks.sweep(); + await t.flush(); + expect(t.hooks.store.lesson(id)).toMatchObject({ status: "dropped" }); + expect(t.hooks.store.openNeeds(binding.id).find((n) => n.kind === "decide" && n.data.lessonId === id)).toBeUndefined(); + }); + + it("Drop on a boundary retire closes the retire pull request and drops the row", async () => { + const t = await adopted(); + const binding = t.hooks.store.find("ENG-12")!; + const recorded = await t.tool("steward_update", { binding: "ENG-12", lesson: { text: "Only the user may grant a credential.", file: "AGENTS.md", scope: "boundary" } }, t.project); + const id = /(lsn_[\w-]+)/u.exec(recorded.text)![1]!; + t.hooks.store.markLessonMerged(id, "d".repeat(40), `refs/heads/guidance-${t.project}`); + t.hooks.store.setLessonPr(id, "https://github.com/acme/bb/pull/7"); + t.hooks.store.setLessonTier(id, "retired"); + await t.tool("steward_update", { lessonAction: { id, action: "retire" } }, t.project); + const need = t.hooks.store.openNeeds(binding.id).find((n) => n.kind === "decide" && n.data.lessonId === id)!; + const dropped = await t.harness.callRpc("stewardsNeedAction", { needId: need.id, action: "choose", choice: "Drop" }, client); + expect(dropped).toMatchObject({ ok: true }); + expect(t.githubClosedPulls).toEqual([{ repo: "acme/bb", number: 1 }]); + expect(t.hooks.store.lesson(id)).toMatchObject({ status: "dropped" }); + expect(t.hooks.store.openNeeds(binding.id).find((n) => n.kind === "decide" && n.data.lessonId === id)).toBeUndefined(); + }); + + it("Drop on one of two lessons sharing a pull request leaves it open for the other", async () => { + const t = await adopted(); + const binding = t.hooks.store.find("ENG-12")!; + const a = await t.tool("steward_update", { binding: "ENG-12", lesson: { text: "Rule A.", file: "AGENTS.md", scope: "boundary" } }, t.project); + const b = await t.tool("steward_update", { binding: "ENG-12", lesson: { text: "Rule B.", file: "AGENTS.md", scope: "boundary" } }, t.project); + const idA = /(lsn_[\w-]+)/u.exec(a.text)![1]!; + const idB = /(lsn_[\w-]+)/u.exec(b.text)![1]!; + await t.tool("steward_update", { lessonAction: { id: idA, action: "complete" } }, t.project); + await t.tool("steward_update", { lessonAction: { id: idB, action: "complete" } }, t.project); + const shared = t.githubPulls[0]!.url; + const needA = t.hooks.store.openNeeds(binding.id).find((n) => n.kind === "decide" && n.data.lessonId === idA)!; + const dropped = await t.harness.callRpc("stewardsNeedAction", { needId: needA.id, action: "choose", choice: "Drop" }, client); + expect(dropped).toMatchObject({ ok: true }); + // The surviving sibling still points at the open PR. + expect(t.githubClosedPulls).toEqual([]); + expect(t.hooks.store.lesson(idA)).toMatchObject({ status: "dropped" }); + expect(t.hooks.store.lesson(idB)).toMatchObject({ status: "pending", pr: shared }); + }); + + it("never merges a boundary lesson's pull request on a project-steward approval, only when it merges", async () => { + const t = await adopted(); + const binding = t.hooks.store.find("ENG-12")!; + const recorded = await t.tool("steward_update", { binding: "ENG-12", lesson: { text: "Only the user may grant a credential.", file: "AGENTS.md", scope: "boundary" } }, t.project); + const id = /(lsn_[\w-]+)/u.exec(recorded.text)![1]!; + await t.tool("steward_update", { lessonAction: { id, action: "complete" } }, t.project); + // A project-steward approval is recorded on the lesson's PR, but it never + // stands in for the owner's merge. + t.hooks.store.recordProjectApproval({ bindingId: binding.id, headSha: "aaaaaaa1", entry: "some entry", byThread: t.project, commentId: "IC_1", verdict: "approved" }); + t.entities.set("acme/bb#1", pull({ id: "acme/bb#1", key: "acme/bb#1" }, { merged: false })); + await t.hooks.sweep(); + await t.flush(); + expect(t.githubMerges).toEqual([]); + expect(t.hooks.store.lesson(id)).toMatchObject({ status: "pending" }); + // Only the owner's merge on the pull request lands it. + t.entities.set("acme/bb#1", pull({ id: "acme/bb#1", key: "acme/bb#1" }, { merged: true })); + await t.hooks.sweep(); + await t.flush(); + expect(t.hooks.store.lesson(id)).toMatchObject({ status: "merged" }); + }); + + it("reviews a boundary lesson's pull request from a thread with bb stewards done --choice Review PR", async () => { + const t = await adopted(); + const recorded = await t.tool("steward_update", { binding: "ENG-12", lesson: { text: "Only the user may grant a credential.", file: "AGENTS.md", scope: "boundary" } }, t.project); + const id = /(lsn_[\w-]+)/u.exec(recorded.text)![1]!; + await t.tool("steward_update", { lessonAction: { id, action: "complete" } }, t.project); + const need = t.hooks.store.openNeeds(t.hooks.store.find("ENG-12")!.id).find((n) => n.kind === "decide")!; + const done = await t.harness.behavior.runCli(["done", need.id, "--choice", "Review PR", "--project", "p1"]); + expect(done.stdout).toMatch(new RegExp(`Review ${t.githubPulls[0]!.url}`, "u")); + expect(t.githubMerges).toEqual([]); + expect(t.hooks.store.lesson(id)).toMatchObject({ status: "pending" }); + }); + + it("Drop closes a boundary lesson's pull request and discards the lesson", async () => { const t = await adopted(); const recorded = await t.tool("steward_update", { binding: "ENG-12", lesson: { text: "Only the user may grant a credential.", file: "AGENTS.md", scope: "boundary" } }, t.project); const id = /(lsn_[\w-]+)/u.exec(recorded.text)![1]!; @@ -6229,8 +6419,9 @@ describe("lessons", () => { const need = t.hooks.store.openNeeds(t.hooks.store.find("ENG-12")!.id).find((n) => n.kind === "decide")!; const dropped = await t.harness.callRpc("stewardsNeedAction", { needId: need.id, action: "choose", choice: "Drop" }, client); expect(dropped).toMatchObject({ ok: true }); + expect(t.githubClosedPulls).toEqual([{ repo: "acme/bb", number: 1 }]); + expect(t.githubMerges).toEqual([]); expect(t.hooks.store.lesson(id)).toMatchObject({ status: "dropped" }); - expect(t.mergeGuidanceCalls).toEqual([]); }); it("a lesson becomes merged when its text is at .bb HEAD on the next pass", async () => { @@ -6307,7 +6498,7 @@ describe("lessons", () => { expect(wake).toContain("## Lessons to land"); expect(wake).toContain(``); await t.tool("steward_update", { lessonAction: { id, action: "complete" } }, t.project); - expect(t.mergeGuidanceCalls.at(-1)).toMatchObject({ file: "AGENTS.md", text: withLessonAnchor(lesson, id) }); + expect(t.githubPulls.at(-1)!.body).toContain(withLessonAnchor(lesson, id)); }); it("retires an aged promoted lesson, and removes its anchored text through the same path", async () => { @@ -6324,7 +6515,9 @@ describe("lessons", () => { expect(wake).toContain("## Lessons to retire"); const done = await t.tool("steward_update", { lessonAction: { id, action: "retire" } }, t.project); expect(done).toMatchObject({ error: false, text: expect.stringMatching(/retired at/u) }); - expect(t.mergeGuidanceCalls.at(-1)).toMatchObject({ file: "AGENTS.md", text: retiredLessonAnchor(id) }); + // A retire is a fresh pull request, not a reuse of the promotion's. + expect(t.githubPulls).toHaveLength(2); + expect(t.githubPulls.at(-1)!.body).toContain(retiredLessonAnchor(id)); expect(t.hooks.store.lesson(id)).toMatchObject({ tier: "retired", status: "dropped" }); }); @@ -6338,9 +6531,10 @@ describe("lessons", () => { expect(wake).not.toContain("A candidate rule."); }); - it("refuses the project file as a lesson's target, so a lesson cannot widen the decide list (BB-35 limit 1)", async () => { + it("refuses the project file and roles.json as a lesson's target, so a lesson cannot widen the decide list (BB-35 limit 1, BB-108)", async () => { const t = await adopted(); await expect(t.tool("steward_update", { lesson: { text: "Move every kind to the project.", file: "stewards/project.json", scope: "procedure" } }, t.project)).rejects.toThrow(/project file/u); + await expect(t.tool("steward_update", { lesson: { text: "Make me the project steward.", file: "stewards/roles.json", scope: "procedure" } }, t.project)).rejects.toThrow(/project file or roles.json/u); }); it("never lands a retired lesson", async () => { @@ -6350,7 +6544,7 @@ describe("lessons", () => { t.hooks.store.setLessonTier(id, "retired"); const out = await t.tool("steward_update", { lessonAction: { id, action: "complete" } }, t.project); expect(out).toMatchObject({ error: true, text: expect.stringMatching(/retired/u) }); - expect(t.mergeGuidanceCalls).toEqual([]); + expect(t.githubPulls).toEqual([]); }); it("drops an aged-out trial that never landed, instead of leaving it pending", async () => { @@ -6361,7 +6555,7 @@ describe("lessons", () => { await t.hooks.nightlyPass("p1", "2026-10-18"); await t.flush(); expect(t.hooks.store.lesson(id)).toMatchObject({ tier: "retired", status: "dropped" }); - expect(t.mergeGuidanceCalls).toEqual([]); + expect(t.githubPulls).toEqual([]); const wake = t.sends.filter((s) => s.threadId === t.project).at(-1)!.text; expect(wake).not.toContain("## Lessons to retire"); }); @@ -6428,7 +6622,7 @@ describe("validation escalations (BB-35 step 5)", () => { // The mechanical pass never promotes a boundary lesson; the user's Merge does. expect(t.hooks.store.lesson(id)).toMatchObject({ tier: "trial", status: "pending" }); const need = validationNeed(t, id)!; - expect(need).toMatchObject({ filedBy: "project", data: { choices: ["Merge", "Drop"], validation: "boundary-promotion" } }); + expect(need).toMatchObject({ filedBy: "project", data: { choices: ["Review PR", "Drop"], validation: "boundary-promotion" } }); // Silence leaves it in trial, which is safe: a trial lesson widens nothing. await t.hooks.nightlyPass("p1", "2026-10-18"); await t.flush(); @@ -6466,31 +6660,30 @@ describe("validation escalations (BB-35 step 5)", () => { expect(validationNeed(t, id)).toBeNull(); }); - it("Retire from the page removes the lesson through the narrow merge", async () => { + it("Retire from the page opens a retire pull request", async () => { const t = await adopted(); const id = await promoted(t); await t.hooks.nightlyPass("p1", "2026-10-18"); await t.flush(); const need = validationNeed(t, id)!; expect(await t.harness.callRpc("stewardsNeedAction", { needId: need.id, action: "choose", choice: "Retire" }, client)).toMatchObject({ ok: true }); - expect(t.mergeGuidanceCalls.at(-1)).toMatchObject({ file: "AGENTS.md", text: retiredLessonAnchor(id) }); + expect(t.githubPulls.at(-1)!.body).toContain(retiredLessonAnchor(id)); expect(t.hooks.store.lesson(id)).toMatchObject({ tier: "retired", status: "dropped" }); expect(validationNeed(t, id)).toBeNull(); }); - it("never merges a lesson that targets the project file (BB-35 limit 1)", async () => { + it("refuses a lesson that targets the project file or roles.json before opening anything (BB-35 limit 1, BB-108)", async () => { const t = await adopted(); - // A row that predates the record-time guard, recorded directly: every - // merge path still refuses it, so it can never move the decide list. - const lesson = t.hooks.store.addLesson({ projectId: "p1", bindingId: t.hooks.store.find("ENG-12")!.id, escalationId: null, file: PROJECT_FILE, text: "Move every kind to the project.", scope: "boundary", branch: null }); - for (const key of ["bnd_0", "bnd_1", "bnd_2"]) t.hooks.store.recordCitations("p1", [lesson.id], key); - t.advance(15 * 86_400_000); - await t.hooks.nightlyPass("p1", "2026-10-18"); - await t.flush(); - const need = validationNeed(t, lesson.id)!; - const merged = await t.harness.callRpc("stewardsNeedAction", { needId: need.id, action: "choose", choice: "Merge" }, client); - expect(merged).toMatchObject({ ok: false, message: expect.stringMatching(/project file/u) }); - expect(t.mergeGuidanceCalls).toEqual([]); + // Rows that predate the record-time guard, recorded directly: the PR path + // still refuses them, so a lesson can never move the decide list or the + // roles split. + for (const file of [PROJECT_FILE, ROLES_FILE]) { + const lesson = t.hooks.store.addLesson({ projectId: "p1", bindingId: t.hooks.store.find("ENG-12")!.id, escalationId: null, file, text: "Move every kind to the project.", scope: "boundary", branch: null }); + const done = await t.tool("steward_update", { lessonAction: { id: lesson.id, action: "complete" } }, t.project); + expect(done).toMatchObject({ error: false, text: expect.stringMatching(/pull request refused.*may never target/u) }); + expect(t.hooks.store.lesson(lesson.id)).toMatchObject({ status: "pending" }); + } + expect(t.githubPulls).toEqual([]); }); it("caps validation escalations at lessons.validations.perWeek and queues the rest", async () => { diff --git a/plugins/stewards/server.ts b/plugins/stewards/server.ts index 83dff42..0f01150 100644 --- a/plugins/stewards/server.ts +++ b/plugins/stewards/server.ts @@ -85,7 +85,7 @@ import { } from "./source-contract.js"; import { DEPLOY_STATES, allDeployed, applyDeployment, applyDeploymentRead, applyWorkflowRun, awaitingOnRepo, deploymentLines, deploymentReportSchema, matchesWorkflow, recordMerged, reportDeployment, timeOut, timeoutsDue } from "./deploy.js"; import { ACTORS, MIGRATIONS, ESCALATED_NEED_KINDS, NEEDS_KINDS, RESOLUTION_KINDS, createStore, needChoices, type Binding, type Escalation, type EscalationRoute, type Fingerprint, type Lesson, type Need, type Observed, type Question, type ReadyState, type Store, type Worker } from "./store.js"; -import { hasLessonAnchor, planLessonTransitions, planValidations, retiredLessonAnchor, withLessonAnchor } from "./lessons.js"; +import { hasLessonAnchor, lessonAnchor, 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 GithubWriter } from "./github.js"; @@ -219,10 +219,15 @@ export function githubRepoOf(url: string): string | null { const readFilesOutput = z .object({ projectSourcePath: z.string().nullable(), head: z.string().nullable(), files: z.array(z.object({ path: z.string(), content: z.string().nullable() }).loose()), error: z.string().nullable() }) .loose(); -const repoSetOutput = z.object({ repos: z.array(z.object({ dir: z.string(), url: z.string() }).loose()), error: z.string().nullable() }).loose(); - -/** multi-repo's answer to a lesson merge, loose like the reads above. */ -const mergeGuidanceOutput = z.object({ ok: z.boolean(), sha: z.string().nullable(), branch: z.string().nullable(), message: z.string() }).loose(); +const repoSetOutput = z + .object({ + repos: z.array(z.object({ dir: z.string(), url: z.string() }).loose()), + /** The project source's own origin, for a lesson's pull request (BB-108). */ + primary: z.object({ dir: z.string(), url: z.string() }).loose().nullable().default(null), + layout: z.enum(["primary", "standalone"]).default("standalone"), + error: z.string().nullable(), + }) + .loose(); /** * The `guidance-` ref a thread's `.bb` commits are published on. @@ -277,6 +282,8 @@ const needViewSchema = z.object({ choices: z.array(z.string()), /** The lesson a `decide` item is about, when it is a boundary lesson; null otherwise. */ lessonId: z.string().nullable(), + /** A boundary lesson's pull request, for the page's "Review PR" link (BB-108); null otherwise. */ + prUrl: z.string().nullable(), what: z.string(), how: z.string().nullable(), /** For a provide-secret item: where the value goes, in words. */ @@ -3652,89 +3659,267 @@ export function createStewardsPlugin(deps: StewardsDeps): (bb: BbPluginApi) => P // // A lesson is a general rule the project steward drew from an escalation // or its own judgement. It is pending until its text is found at `.bb` - // HEAD. A `procedure` lesson the project steward applies and publishes is - // self-merged through multi-repo; a `boundary` one changes its own - // escalation boundary and is filed for the user with Merge/Drop, never - // merged by the plugin. - - /** Close the open Merge/Drop item for a boundary lesson, if one is filed. */ + // HEAD. Since BB-106 made `.bb` a directory on the primary repo's default + // branch, a promoted or retired lesson lands as a reviewed pull request + // (BB-108): the project steward publishes the `guidance-` branch, + // and this plugin opens the pull request. A `procedure` lesson's PR is + // merged by the project steward at once; a `boundary` one waits for the + // owner to merge it, and the lesson is marked merged when the PR merges. + + /** Close the open Review PR/Drop item for a boundary lesson, if one is filed. */ function closeLessonNeed(lesson: Lesson, resolution: string): void { if (lesson.bindingId === null) return; store.closeNeedByKey(lesson.bindingId, "decide", lesson.id, resolution); } - /** Self-merge a procedure lesson, or file a boundary one for the user. */ + /** The files a lesson may never target: the decide list, the roles split, and anything under them. */ + function lessonFileAllowed(file: string): boolean { + return file !== PROJECT_FILE && file !== ROLES_FILE; + } + + /** + * The primary repo a lesson's pull request opens on, from multi-repo's repo + * set. + * + * TODO(BB-107): when stewards has a native git project source, this must + * read the primary repo from that seam instead, and the publish in + * `workspace_publish_guidance` must be replaced by the native handler. Until + * BB-107 lands, a project with no multi-repo reports the lesson PR refused + * rather than opening one; the branch push is multi-repo's. + */ + async function lessonRepo(projectId: string): Promise<{ repo: string } | { error: string }> { + let set: z.infer; + try { + set = await rpc(MULTI_REPO_PLUGIN_ID, "repoSet", { projectId }, repoSetOutput); + } catch (error) { + bb.log.warn(`stewards: could not read the repo set for a lesson pull request: ${errorMessage(error)}`); + return { error: "could not read the project's primary repo; is multi-repo installed?" }; + } + if (set.primary === null) return { error: "the project source has no origin to open a lesson pull request on" }; + const repo = githubRepoOf(set.primary.url); + if (repo === null) return { error: `the project source's origin ${set.primary.url} is not a GitHub repo` }; + return { repo }; + } + + /** The signed body of a lesson's pull request: the id, the anchor, and the steward that proposed it. */ + function lessonPrBody(lesson: Lesson, text: string, threadId: string, retire: boolean): string { + const binding = lesson.bindingId === null ? null : store.binding(lesson.bindingId); + const anchor = retire ? retiredLessonAnchor(lesson.id) : lessonAnchor(lesson.id); + return [ + `${retire ? "Retires" : "Lands"} lesson \`${lesson.id}\` in \`${lesson.file}\` (proposed by the project steward${binding === null ? "" : ` for ${binding.entityKey}`}).`, + "", + `Anchor: \`${anchor}\``, + "", + text, + "", + `_— from bb thread ${threadId}_`, + ].join("\n"); + } + + type LessonPrResult = { ok: boolean; repo: string | null; url: string | null; number: number | null; branch: string | null; message: string }; + + /** + * Open the pull request that lands a lesson's anchored block on the primary + * repo. The branch has already been published by + * `workspace_publish_guidance`; this only turns it into a PR. The + * self-widening guard (BB-35 limit 1) is enforced here, before anything is + * opened, and the same branch is reused when a nightly pass lands several + * lessons in one publish. A retirement asks for `fresh` so the promotion's + * merged PR is never reused. + */ + async function openLessonPr(lesson: Lesson, text: string, opts: { retire?: boolean } = {}): Promise { + const retire = opts.retire === true; + const projectThreadId = store.project(lesson.projectId).threadId; + const branch = lesson.branch ?? (projectThreadId === null ? null : lessonBranchFor(projectThreadId)); + if (!lessonFileAllowed(lesson.file)) return { ok: false, repo: null, url: null, number: null, branch, message: `a lesson may never target ${lesson.file}` }; + if (projectThreadId === null) return { ok: false, repo: null, url: null, number: null, branch, message: "the project steward has no thread yet, so nothing has been published" }; + if (branch === null) return { ok: false, repo: null, url: null, number: null, branch, message: "the lesson has no branch to open a pull request from" }; + if (lesson.branch === null) store.setLessonBranch(lesson.id, branch); + const primary = await lessonRepo(lesson.projectId); + if ("error" in primary) return { ok: false, repo: null, url: null, number: null, branch, message: primary.error }; + const already = retire ? lesson.retirePr : lesson.pr; + if (already !== null) { + const ref = parsePullRef(already); + return { ok: true, repo: primary.repo, url: already, number: ref?.number ?? null, branch, message: `already open at ${already}` }; + } + if (!retire) { + // Several lessons landed in one nightly publish share a branch, so they + // share one pull request. + const sibling = store.allLessons(lesson.projectId).find((l) => l.id !== lesson.id && l.status === "pending" && l.branch === branch && l.pr !== null); + if (sibling !== undefined && sibling.pr !== null) { + store.setLessonPr(lesson.id, sibling.pr); + const ref = parsePullRef(sibling.pr); + return { ok: true, repo: primary.repo, url: sibling.pr, number: ref?.number ?? null, branch, message: `reuses ${sibling.pr}` }; + } + } + const base = await github.defaultBranch(primary.repo).catch((error: unknown) => { + bb.log.warn(`stewards: could not read ${primary.repo}'s default branch: ${errorMessage(error)}`); + return null; + }); + if (base === null) return { ok: false, repo: primary.repo, url: null, number: null, branch, message: `could not read ${primary.repo}'s default branch` }; + const head = branch.replace(/^refs\/heads\//u, ""); + try { + const created = await github.createPull(primary.repo, { head, base, title: `${retire ? "Retire" : "Lesson"} ${lesson.id}: ${lesson.file}`, body: lessonPrBody(lesson, text, projectThreadId, retire) }); + if (retire) store.setLessonRetirePr(lesson.id, created.url); + else store.setLessonPr(lesson.id, created.url); + return { ok: true, repo: primary.repo, url: created.url, number: created.number, branch, message: `opened ${created.url}` }; + } catch (error) { + return { ok: false, repo: primary.repo, url: null, number: null, branch, message: errorMessage(error) }; + } + } + + /** + * The `.bb` files the pull request may change: this lesson's own file and + * the files of the lessons that share its PR (several lessons in one + * publish). The project file and `roles.json` are never allowed, so a + * hand-edited pre-guard row cannot widen the set. + */ + function lessonPrFiles(projectId: string, url: string, lessonId: string): string[] { + return [ + ...new Set( + store + .allLessons(projectId) + .filter((l) => (l.id === lessonId || l.pr === url || l.retirePr === url) && lessonFileAllowed(l.file)) + .map((l) => l.file), + ), + ]; + } + + /** + * The narrow guard the old fast-forward enforced, on the pull request + * (BB-35 limit 1, BB-108): the PR must change only the files of the lessons + * published on its branch, and this lesson's file must carry its anchor. + * Returns why it refuses, or null when the PR is safe to merge. Fails + * closed when the read is unavailable. + */ + async function verifyLessonPr(lesson: Lesson, result: LessonPrResult, anchor: string): Promise { + if (result.repo === null || result.number === null || result.branch === null) return "the pull request could not be read"; + if (github.files === undefined) return "the pull request's changed files cannot be read"; + let files: string[]; + try { + files = await github.files(result.repo, result.number); + } catch (error) { + return `could not read the pull request's changed files: ${errorMessage(error)}`; + } + // `files` reads one page; a full page means there may be more to check. + if (files.length >= 100) return "the pull request changes too many files to verify"; + const allowed = lessonPrFiles(lesson.projectId, result.url ?? "", lesson.id); + const bad = files.filter((path) => !allowed.some((file) => path === file || path === `.bb/${file}`)); + if (bad.length > 0) return `the pull request changes files other than the lesson's own: ${bad.slice(0, 5).join(", ")}`; + const path = files.find((p) => p === lesson.file || p === `.bb/${lesson.file}`); + if (path === undefined) return `the pull request does not change ${lesson.file}`; + if (github.fileContent === undefined) return `${path} cannot be read from the pull request`; + let content: string | null; + try { + content = await github.fileContent(result.repo, path, result.branch.replace(/^refs\/heads\//u, "")); + } catch (error) { + return `could not read ${path} from the pull request: ${errorMessage(error)}`; + } + if (content === null || !content.includes(anchor)) return `${path} does not carry the lesson's anchor`; + 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. */ + 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; + const problem = await verifyLessonPr(lesson, result, retire ? retiredLessonAnchor(lesson.id) : lessonAnchor(lesson.id)); + if (problem !== null) { + store.noteLesson(lesson.id, problem); + return { ok: false, message: `pull request refused: ${problem}` }; + } + try { + const { sha } = await github.squashMerge(result.repo, result.number, `Lesson ${lesson.id}`, note); + store.markLessonMerged(lesson.id, sha, result.branch ?? lesson.branch, note); + return { ok: true, message: `${opts.verb ?? "merged"} at ${sha ?? result.url ?? "?"}${result.branch === null ? "" : ` on ${result.branch}`}` }; + } catch (error) { + const message = errorMessage(error); + store.noteLesson(lesson.id, message); + return { ok: false, message: `pull request refused: ${message}` }; + } + } + + /** + * Close a lesson's open pull request (Drop). A promotion pull request can + * be shared by several pending lessons in one publish, so it is closed only + * when no other pending lesson still references it; the survivors keep it. + */ + async function closeLessonPr(lesson: Lesson, url?: string | null): Promise { + const pr = url === undefined ? (lesson.retirePr ?? lesson.pr) : url; + if (pr === null) return; + const shared = store.allLessons(lesson.projectId).some((l) => l.id !== lesson.id && l.status === "pending" && (l.pr === pr || l.retirePr === pr)); + if (shared) return; + const ref = parsePullRef(pr); + if (ref === null) return; + await github.closePull(ref.repo, ref.number).catch((error: unknown) => bb.log.warn(`stewards: could not close ${pr}: ${errorMessage(error)}`)); + } + + /** Land a procedure lesson at once, or file a boundary one for the owner to review. */ async function completeLesson(lesson: Lesson, action: "complete" | "drop"): Promise { if (action === "drop") { + await closeLessonPr(lesson); store.markLessonDropped(lesson.id, "Dropped by the project steward."); closeLessonNeed(lesson, "Dropped by the project steward."); return "dropped"; } - const threadId = store.project(lesson.projectId).threadId; - if (lesson.scope === "boundary") { - if (lesson.bindingId === null) return "a boundary lesson needs the binding it came from; record it with binding."; - const branch = lesson.branch ?? (threadId === null ? null : lessonBranchFor(threadId)); - if (branch !== null) store.setLessonBranch(lesson.id, branch); - fileBoundaryLessonNeed(lesson, branch); - return `boundary lesson; Merge/Drop filed for the user${branch === null ? "" : ` on ${branch}`}`; - } - if (threadId === null) { - store.noteLesson(lesson.id, "The project steward has no thread yet, so nothing has been published."); - return "no project steward thread"; - } - // The file carries the lesson's anchor, so the merge's narrow guard can - // check the right block landed and the same lesson cannot land twice. - const result = await mergeLesson(lesson.projectId, threadId, lesson, withLessonAnchor(lesson.text, lesson.id)); + if (lesson.scope === "boundary" && lesson.bindingId === null) return "a boundary lesson needs the binding it came from; record it with binding."; + // A publish that landed several lessons in one branch may already have + // put this one's anchored text at HEAD when an earlier lesson merged; + // observe it rather than opening a pull request with no commits. + const atHead = (await readFiles(lesson.projectId, [lesson.file]).catch(() => new Map())).get(lesson.file) ?? null; + if (atHead !== null && hasLessonAnchor(atHead, lesson.id)) { + store.markLessonMerged(lesson.id, lesson.mergeSha, lesson.branch, "Already at .bb HEAD."); + closeLessonNeed(lesson, "Already at .bb HEAD."); + return "already landed at .bb HEAD"; + } + const result = await openLessonPr(lesson, withLessonAnchor(lesson.text, lesson.id)); if (!result.ok) { store.noteLesson(lesson.id, result.message); - return `merge refused: ${result.message}`; + return `pull request refused: ${result.message}`; } - store.markLessonMerged(lesson.id, result.sha, result.branch ?? lesson.branch); - return `merged at ${result.sha ?? "?"}${result.branch === null ? "" : ` on ${result.branch}`}`; + if (lesson.scope === "boundary") { + fileBoundaryLessonNeed(lesson, result.branch, result.url); + return `boundary lesson; Review PR/Drop filed for the user at ${result.url ?? "?"}`; + } + return (await mergeLessonPr(lesson, result, "Landed by the project steward (procedure lesson).")).message; } /** - * Remove a retired lesson's text from `.bb` through the same narrow merge - * path, leaving the retired anchor behind. The project steward has already - * removed the text and published the branch; this fast-forwards it. A - * lesson that never reached the file is retired in the row alone. + * Remove a retired lesson's text from `.bb` as a pull request, leaving the + * retired anchor behind. The project steward has already removed the text + * and published the branch. A lesson that never reached the file is retired + * in the row alone. */ async function retireLesson(lesson: Lesson): Promise { if (lesson.status !== "merged") { store.markLessonDropped(lesson.id, "Retired before it reached the file."); return "retired (it was not in the file)"; } - const threadId = store.project(lesson.projectId).threadId; - if (threadId === null) { - store.noteLesson(lesson.id, "The project steward has no thread yet, so nothing could be removed."); - return "no project steward thread"; + // A publish that retired several lessons in one branch may already have + // put this one's retired anchor at HEAD when an earlier retire merged. + const atHead = (await readFiles(lesson.projectId, [lesson.file]).catch(() => new Map())).get(lesson.file) ?? null; + if (atHead !== null && atHead.includes(retiredLessonAnchor(lesson.id))) { + store.markLessonDropped(lesson.id, `Retired; the anchored text was removed from ${lesson.file}.`); + closeLessonNeed(lesson, "Already retired at .bb HEAD."); + return "already retired at .bb HEAD"; } - const result = await mergeLesson(lesson.projectId, threadId, lesson, retiredLessonAnchor(lesson.id)); + const result = await openLessonPr(lesson, retiredLessonAnchor(lesson.id), { retire: true }); if (!result.ok) { store.noteLesson(lesson.id, result.message); return `retire refused: ${result.message}`; } - store.markLessonDropped(lesson.id, `Retired; the anchored text was removed from ${lesson.file}.`); - return `retired at ${result.sha ?? "?"}${result.branch === null ? "" : ` on ${result.branch}`}`; - } - - /** Fast-forward the canonical `.bb` to a lesson's published branch, through multi-repo. */ - async function mergeLesson(projectId: string, threadId: string, lesson: Lesson, text: string): Promise<{ ok: boolean; sha: string | null; branch: string | null; message: string }> { - // Defence in depth for the self-widening guard (BB-35 limit 1): the tool - // refuses a project-file lesson, and every merge path refuses one too, so - // a row that predates the guard can never move the decide list. - if (lesson.file === PROJECT_FILE) return { ok: false, sha: null, branch: lesson.branch, message: "a lesson may never target the project file" }; - try { - return await rpc(MULTI_REPO_PLUGIN_ID, "mergeGuidance", { projectId, threadId, file: lesson.file, text }, mergeGuidanceOutput); - } catch (error) { - return { ok: false, sha: null, branch: lesson.branch, message: errorMessage(error) }; + if (lesson.scope === "boundary") { + fileBoundaryLessonNeed(lesson, result.branch, result.url, true); + return `boundary retire; Review PR/Drop filed for the user at ${result.url ?? "?"}`; } + const merged = await mergeLessonPr(lesson, result, `Retired; the anchored text was removed from ${lesson.file}.`, { retire: true, verb: "retired" }); + if (merged.ok) store.markLessonDropped(lesson.id, `Retired; the anchored text was removed from ${lesson.file}.`); + return merged.message; } /** Each pass, a lesson whose anchored text is already at `.bb` HEAD is merged, however it got there. */ async function reconcileLessons(config: ProjectConfig): Promise { const pending = store.lessons(config.projectId).filter((lesson) => lesson.tier !== "retired"); - if (pending.length === 0) return; for (const lesson of pending) { // One lesson's file being unreadable — a bad path, a transient read // failure — must not stop every other lesson being checked. @@ -3749,6 +3934,39 @@ export function createStewardsPlugin(deps: StewardsDeps): (bb: BbPluginApi) => P // wording, so a plain-text match still counts. if (content !== null && (hasLessonAnchor(content, lesson.id) || normalizeLessonText(content).includes(normalizeLessonText(lesson.text)))) { store.markLessonMerged(lesson.id, lesson.mergeSha, lesson.branch); + closeLessonNeed(lesson, "The anchored text is at .bb HEAD."); + } + } + await reconcileLessonPrs(config); + } + + /** + * A pending lesson's pull request is the merge signal (BB-108): read it + * through the github source and mark the lesson merged when it merged, + * closing the owner's Review PR item. The read is best-effort, so one + * unreadable pull request does not stop the others. + */ + async function reconcileLessonPrs(config: ProjectConfig): Promise { + for (const lesson of store.allLessons(config.projectId)) { + // A pending lesson lands when its promotion PR merges; a retired lesson + // whose anchored text is still in the file (status merged) is dropped + // when its retire PR merges. The two PRs are kept apart so a promotion + // merge never retires a lesson and a retire merge never lands one. + const retire = lesson.status === "merged" && lesson.retirePr !== null; + if (!retire && lesson.status !== "pending") continue; + const pr = retire ? lesson.retirePr : lesson.pr; + if (pr === null) continue; + const ref = parsePullRef(pr); + if (ref === null) continue; + try { + const pull = await snapshot("github", `${ref.repo}#${ref.number}`); + if (pull?.pull?.merged === true) { + if (retire) store.markLessonDropped(lesson.id, `Retired; the anchored text was removed from ${lesson.file}.`); + else store.markLessonMerged(lesson.id, lesson.mergeSha, lesson.branch, "The pull request merged."); + closeLessonNeed(lesson, "The pull request merged."); + } + } catch (error) { + bb.log.warn(`stewards: could not read lesson ${lesson.id}'s pull request ${pr}: ${errorMessage(error)}`); } } } @@ -3799,26 +4017,29 @@ export function createStewardsPlugin(deps: StewardsDeps): (bb: BbPluginApi) => P } /** - * File the boundary lesson's Merge/Drop item (BB-35 steps 3/5). It is the - * existing validation escalation: only the user's Merge makes a boundary - * lesson permanent, and an unanswered item leaves the lesson in trial. + * 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. */ - function fileBoundaryLessonNeed(lesson: Lesson, branch: string | null): boolean { + function fileBoundaryLessonNeed(lesson: Lesson, branch: string | null, prUrl: string | null, retire = false): boolean { if (lesson.bindingId === null) return false; store.fileNeed({ bindingId: lesson.bindingId, kind: "decide", url: lesson.id, - what: `Merge or drop the boundary lesson for ${lesson.file}`, + what: `Review or drop the ${retire ? "retirement of the " : ""}boundary lesson for ${lesson.file}`, how: [ - "The project steward recorded a general rule that changes its own escalation boundary, so only you may make it permanent.", + 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.", "", `> ${lesson.text}`, "", `File: \`${lesson.file}\`. Branch: \`${branch ?? "(not published yet)"}\`.`, - "Merge fast-forwards the canonical `.bb`; Drop discards it.", + prUrl === null ? "The pull request is not open yet." : `Review the pull request at ${prUrl}; the lesson is marked ${retire ? "dropped" : "merged"} when it merges. Drop closes it and discards the lesson.`, ].join("\n"), - data: { lessonId: lesson.id, branch, file: lesson.file, choices: ["Merge", "Drop"], validation: "boundary-promotion" }, + data: { lessonId: lesson.id, branch, file: lesson.file, prUrl, retire, choices: ["Review PR", "Drop"], validation: "boundary-promotion" }, filedBy: "project", }); return true; @@ -3877,7 +4098,7 @@ export function createStewardsPlugin(deps: StewardsDeps): (bb: BbPluginApi) => P if (remaining <= 0) continue; const branch = request.validation === "boundary-promotion" ? lessonBranch(lesson) : null; if (branch !== null) store.setLessonBranch(lesson.id, branch); - const filed = request.validation === "boundary-promotion" ? fileBoundaryLessonNeed(lesson, branch) : fileMixedEvidenceNeed(lesson); + const filed = request.validation === "boundary-promotion" ? fileBoundaryLessonNeed(lesson, branch, lesson.pr) : fileMixedEvidenceNeed(lesson); if (!filed) { // A lesson with no binding (a hand-edited or pre-guard row) has no // item to live on; say so once rather than skipping it in silence. @@ -5532,15 +5753,18 @@ export function createStewardsPlugin(deps: StewardsDeps): (bb: BbPluginApi) => P return { ok: true, message: `Opening the secure form for ${named} in a new thread.`, threadId: requesterId }; } - // A lesson's validation item (BB-35 steps 3/5). A boundary one is - // Merge/Drop: Merge goes through the same multi-repo fast-forward the - // project steward's own self-merge uses; Drop discards it. A + // A lesson's validation item (BB-35 steps 3/5, BB-108). A boundary one + // is Review PR/Drop: the pull request lands it and Drop closes it. A // mixed-evidence one is Keep/Retire: Keep records that the user // confirmed it, Retire removes it. Neither is a free-text answer. if (need.kind === "decide" && action === "choose" && typeof need.data.lessonId === "string") { const lesson = store.lesson(need.data.lessonId); const label = choice === undefined || choice === null || choice === "" ? null : choice; - if (lesson === null || lesson.tier === "retired") { + // A retire item is about a retired lesson whose text is still in the + // file (status merged, a retire PR open); every other lesson item is + // about a pending lesson. Anything else is already resolved. + const isRetire = lesson !== null && lesson.retirePr !== null && lesson.status === "merged"; + if (lesson === null || lesson.status === "dropped" || (lesson.tier === "retired" && !isRetire)) { store.closeNeedByKey(b.id, "decide", need.url, "The lesson is already resolved."); return { ok: false, message: "That lesson is already resolved." }; } @@ -5560,28 +5784,20 @@ export function createStewardsPlugin(deps: StewardsDeps): (bb: BbPluginApi) => P after(`The user retired lesson ${lesson.id} (${lesson.file}) on the Stewards page: ${removed}.`); return { ok: true, message: `Retired; ${b.entityKey}'s steward has been told.` }; } - if (lesson.status !== "pending") { + if (!isRetire && lesson.status !== "pending") { store.closeNeedByKey(b.id, "decide", need.url, "The lesson is already resolved."); return { ok: false, message: "That lesson is already resolved." }; } if (label === "Drop") { - store.markLessonDropped(lesson.id, "Dropped by the user on the Stewards page."); + await closeLessonPr(lesson, isRetire ? lesson.retirePr : lesson.pr); + store.markLessonDropped(lesson.id, isRetire ? "Retire dropped by the user on the Stewards page." : "Dropped by the user on the Stewards page."); store.closeNeedByKey(b.id, "decide", need.url, "Dropped on the Stewards page."); after(`The user dropped lesson ${lesson.id} on the Stewards page.`); return { ok: true, message: `Dropped; ${b.entityKey}'s steward has been told.` }; } - if (label !== "Merge") return { ok: false, message: `"${label ?? ""}" is not one of the choices: Merge, Drop.` }; - const threadId = store.project(b.projectId).threadId; - if (threadId === null) return { ok: false, message: "The project steward has no thread yet; nothing has been published." }; - const result = await mergeLesson(b.projectId, threadId, lesson, withLessonAnchor(lesson.text, lesson.id)); - if (!result.ok) { - store.noteLesson(lesson.id, result.message); - return { ok: false, message: result.message }; - } - store.markLessonMerged(lesson.id, result.sha, result.branch ?? lesson.branch, "Merged by the user on the Stewards page."); - store.closeNeedByKey(b.id, "decide", need.url, "Merged on the Stewards page."); - after(`The user merged lesson ${lesson.id} (${lesson.file}) on the Stewards page at ${result.sha ?? "?"}.`); - return { ok: true, message: `Merged; ${b.entityKey}'s steward has been told.` }; + if (label !== "Review PR") return { ok: false, message: `"${label ?? ""}" is not one of the choices: Review PR, Drop.` }; + const prUrl = typeof need.data.prUrl === "string" ? need.data.prUrl : (lesson.retirePr ?? lesson.pr); + return { ok: true, message: prUrl === null ? "The pull request is not open yet; the project steward has not landed it." : `Review ${prUrl}; the lesson is marked ${isRetire ? "dropped" : "merged"} when it merges.` }; } if (need.kind === "decide" && action === "choose") { @@ -6289,7 +6505,7 @@ export function createStewardsPlugin(deps: StewardsDeps): (bb: BbPluginApi) => P (f) => !f.startsWith("/") && !f.includes("\\") && !f.includes("\0") && f.split("/").every((s) => s !== ".." && s !== "." && s.length > 0), "a .bb-relative path with no leading /, backslash, NUL, or empty/./.. segment", ) - .refine((f) => f !== "stewards/project.json", "a guidance file (AGENTS.md or a playbook), never the project file: a lesson must not be able to move a big decision to the project, loosen the pull-request gate or relax an approval") + .refine((f) => f !== PROJECT_FILE && f !== ROLES_FILE, "a guidance file (AGENTS.md or a playbook), never the project file or roles.json: a lesson must not be able to move a big decision to the project, loosen the pull-request gate, relax an approval or change who is a steward") .describe("The .bb file it belongs in, repo-relative, such as AGENTS.md or stewards/linear-tickets.md."), scope: z.enum(["procedure", "boundary"]).describe("procedure: playbook wording, sequencing, conventions the steward may already decide, which it self-merges. boundary: credentials and reach, who may direct a server, irreversible production steps — the user decides."), supersedes: z.string().trim().min(1).max(100).optional().describe("An existing lesson id in the same file that this one replaces; the nightly pass retires the older one."), @@ -6299,7 +6515,7 @@ export function createStewardsPlugin(deps: StewardsDeps): (bb: BbPluginApi) => P lessonAction: z .object({ id: z.string().trim().min(1).max(100).describe("The lesson id (lsn_…) from the nightly prompt or `bb stewards lessons`."), - action: z.enum(["complete", "drop", "retire"]).describe("complete: self-merge a procedure lesson, or file the user's Merge/Drop item for a boundary one. retire: remove a retired lesson's anchored text from `.bb`. drop: discard it."), + action: z.enum(["complete", "drop", "retire"]).describe("complete: open the lesson's pull request and merge it for a procedure lesson, or file the owner's Review PR/Drop item for a boundary one. retire: open a pull request removing a retired lesson's anchored text. drop: close the pull request and discard it."), }) .optional() .describe("Project steward only: finish a pending lesson after applying and publishing it, retire a retired one, or drop it."), @@ -6435,7 +6651,7 @@ export function createStewardsPlugin(deps: StewardsDeps): (bb: BbPluginApi) => P if (closing === null && input.ask === undefined && input.lesson === undefined) return toolError(new Error("Nothing to update: name a binding, ask a question, record a lesson, or close one.")); const written = questionWrites(closing, null); if (input.lesson !== undefined) { - if (input.lesson.scope === "boundary") return toolError(new Error("A boundary lesson must name the binding it came from: pass binding (and resolve) so the user's Merge/Drop item has somewhere to live.")); + if (input.lesson.scope === "boundary") return toolError(new Error("A boundary lesson must name the binding it came from: pass binding (and resolve) so the owner's Review PR/Drop item has somewhere to live.")); written.push(recordLesson(null, closing?.question.escalationId ?? null)); } return `${written.join("; ")}.`; @@ -6783,7 +6999,7 @@ export function createStewardsPlugin(deps: StewardsDeps): (bb: BbPluginApi) => P }); /** One open "needs you" item, with whether its button can act now and why not when it cannot. */ - function needView(n: Need, b: Binding): { id: string; bindingId: string; key: string; kind: string; url: string | null; entityUrl: string | null; escalationId: string | null; choices: string[]; lessonId: string | null; what: string; how: string | null; destination: string | null; threadId: string | null; createdAt: number; actionable: boolean; why: string | null } { + function needView(n: Need, b: Binding): { id: string; bindingId: string; key: string; kind: string; url: string | null; entityUrl: string | null; escalationId: string | null; choices: string[]; lessonId: string | null; prUrl: string | null; what: string; how: string | null; destination: string | null; threadId: string | null; createdAt: number; actionable: boolean; why: string | null } { let actionable = true; let why: string | null = null; let threadId: string | null = null; @@ -6804,7 +7020,7 @@ export function createStewardsPlugin(deps: StewardsDeps): (bb: BbPluginApi) => P // appears in its first turn. Link the thread once one has been spawned. threadId = store.secretRequesterForNeed(n.id)?.threadId ?? null; } - return { id: n.id, bindingId: b.id, key: b.entityKey, kind: n.kind, url: /^https?:\/\//iu.test(n.url) ? n.url : null, entityUrl: b.url, escalationId: typeof n.data.escalationId === "string" ? n.data.escalationId : null, choices: needChoices(n), lessonId: typeof n.data.lessonId === "string" ? n.data.lessonId : null, what: n.what, how: n.how, destination: n.kind === "provide-secret" ? secretDestination(n) : null, threadId, createdAt: n.createdAt, actionable, why }; + return { id: n.id, bindingId: b.id, key: b.entityKey, kind: n.kind, url: /^https?:\/\//iu.test(n.url) ? n.url : null, entityUrl: b.url, escalationId: typeof n.data.escalationId === "string" ? n.data.escalationId : null, choices: needChoices(n), lessonId: typeof n.data.lessonId === "string" ? n.data.lessonId : null, prUrl: typeof n.data.prUrl === "string" ? n.data.prUrl : null, what: n.what, how: n.how, destination: n.kind === "provide-secret" ? secretDestination(n) : null, threadId, createdAt: n.createdAt, actionable, why }; } /** @@ -7504,7 +7720,8 @@ export function createStewardsPlugin(deps: StewardsDeps): (bb: BbPluginApi) => P // wakes the steward. if (n.kind === "decide" && typeof n.data.lessonId === "string") { const lesson = store.lesson(n.data.lessonId); - if (lesson === null || lesson.tier === "retired") return { exitCode: 0, stdout: "That lesson is already resolved." }; + const isRetire = lesson !== null && lesson.retirePr !== null && lesson.status === "merged"; + if (lesson === null || lesson.status === "dropped" || (lesson.tier === "retired" && !isRetire)) return { exitCode: 0, stdout: "That lesson is already resolved." }; const label = choice ?? null; if (n.data.validation === "mixed-evidence") { if (label !== "Keep" && label !== "Retire") throw new PluginCliError(`Lesson ${lesson.id} is Keep or Retire; pass --choice Keep or --choice Retire.`, { code: "choice_required" }); @@ -7527,11 +7744,12 @@ export function createStewardsPlugin(deps: StewardsDeps): (bb: BbPluginApi) => P } return { exitCode: 0, stdout: `Retired ${lesson.id}.` }; } - if (lesson.status !== "pending") return { exitCode: 0, stdout: "That lesson is already resolved." }; - if (label !== "Merge" && label !== "Drop") throw new PluginCliError(`Lesson ${lesson.id} is Merge or Drop; pass --choice Merge or --choice Drop.`, { code: "choice_required" }); + if (!isRetire && lesson.status !== "pending") return { exitCode: 0, stdout: "That lesson is already resolved." }; + if (label !== "Review PR" && label !== "Drop") throw new PluginCliError(`Lesson ${lesson.id} is Review PR or Drop; pass --choice Review PR or --choice Drop.`, { code: "choice_required" }); const b = store.binding(n.bindingId); if (label === "Drop") { - store.markLessonDropped(lesson.id, "Dropped from a thread."); + await closeLessonPr(lesson, isRetire ? lesson.retirePr : lesson.pr); + store.markLessonDropped(lesson.id, isRetire ? "Retire dropped from a thread." : "Dropped from a thread."); store.closeNeedByKey(n.bindingId, "decide", n.url, "Dropped from a thread."); if (b !== null) { store.note(b.id, `The user dropped lesson ${lesson.id} from a thread.`); @@ -7539,20 +7757,8 @@ export function createStewardsPlugin(deps: StewardsDeps): (bb: BbPluginApi) => P } return { exitCode: 0, stdout: `Dropped ${lesson.id}.` }; } - const threadId = b === null ? null : store.project(b.projectId).threadId; - if (threadId === null) throw new PluginCliError("The project steward has no thread yet; nothing has been published.", { code: "no_project_steward" }); - const result = await mergeLesson(b!.projectId, threadId, lesson, withLessonAnchor(lesson.text, lesson.id)); - if (!result.ok) { - store.noteLesson(lesson.id, result.message); - throw new PluginCliError(result.message, { code: "merge_refused" }); - } - store.markLessonMerged(lesson.id, result.sha, result.branch ?? lesson.branch, "Merged from a thread."); - store.closeNeedByKey(n.bindingId, "decide", n.url, "Merged from a thread."); - if (b !== null) { - store.note(b.id, `The user merged lesson ${lesson.id} from a thread at ${result.sha ?? "?"}.`); - scheduleReconcile(b.id); - } - return { exitCode: 0, stdout: `Merged ${lesson.id}.` }; + const prUrl = typeof n.data.prUrl === "string" ? n.data.prUrl : (lesson.retirePr ?? lesson.pr); + return { exitCode: 0, stdout: prUrl === null ? `The pull request for ${lesson.id} is not open yet.` : `Review ${prUrl}` }; } if (choice !== undefined) { if (n.kind !== "decide") throw new PluginCliError(`Choices are for a decide item; ${id} is ${n.kind}.`, { code: "not_decide" }); diff --git a/plugins/stewards/skills/stewards/SKILL.md b/plugins/stewards/skills/stewards/SKILL.md index 7bfc401..4eb4769 100644 --- a/plugins/stewards/skills/stewards/SKILL.md +++ b/plugins/stewards/skills/stewards/SKILL.md @@ -340,21 +340,30 @@ corrections, 14 days) or retires it (30 days uncited, or a correction); a promoted lesson is retired at 90 days uncited or 2 corrections. The thresholds, caps and decay half-life live under `lessons` in `stewards/project.json`. A `procedure` lesson is promoted mechanically; a -`boundary` one is not — the pass files the user's Merge/Drop item and the -lesson stays `trial` until they Merge. Two rare validation escalations exist, -capped at `lessons.validations.perWeek` (default 1) per rolling week and -ordered by score: the boundary Merge/Drop, and one Keep/Retire question for a -promoted lesson with mixed evidence (citations and a correction short of the -retire count). Silence has a default: an unanswered boundary item leaves the -lesson in trial, while an unanswered Keep/Retire retires it on the next -nightly pass; a Keep is remembered, so it is never asked again. When the pass queues a promotion or a retirement +`boundary` one is not — the pass files the owner's Review PR/Drop item and the +lesson stays `trial` until the pull request merges. Two rare validation +escalations exist, capped at `lessons.validations.perWeek` (default 1) per +rolling week and ordered by score: the boundary Review PR/Drop, and one +Keep/Retire question for a promoted lesson with mixed evidence (citations and +a correction short of the retire count). Silence has a default: an unanswered +boundary item leaves the lesson in trial, while an unanswered Keep/Retire +retires it on the next nightly pass; a Keep is remembered, so it is never +asked again. When the pass queues a promotion or a retirement your wake lists it under `## Lessons to land` or `## Lessons to retire`: add or replace the anchored block (``) in the named -file, commit, publish with `workspace_publish_guidance`, then +file, publish with `workspace_publish_guidance`, then `steward_update { lessonAction: { id, action: "complete" } }` or `retire`. -A lesson may never target `stewards/project.json`, so it cannot widen the -decide list, loosen the pull-request gate or relax an approval; a boundary -lesson still goes to the user as Merge/Drop. Every wake lists the questions still open: list them from +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 +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 +Review PR/Drop. Every wake lists the questions still open: list them from there, never from memory, and do not ask one again. At the end of each nightly pass your reply is kept as the morning digest; the wake carries the record to write it from diff --git a/plugins/stewards/store.test.ts b/plugins/stewards/store.test.ts index 0249f59..d93501e 100644 --- a/plugins/stewards/store.test.ts +++ b/plugins/stewards/store.test.ts @@ -1,10 +1,10 @@ import Database from "better-sqlite3"; import { describe, expect, it } from "vitest"; import { z } from "zod"; -import { ACTOR_MIGRATIONS, APPROVAL_MARKER_MIGRATIONS, BASE_MIGRATIONS, CHAIN_MIGRATIONS, GUIDANCE_MIGRATIONS, IDLE_MIGRATIONS, LESSON_COUNTER_MIGRATIONS, LESSON_TIER_MIGRATIONS, MIGRATIONS, RATIONALE_MIGRATIONS, VALIDATION_MIGRATIONS, WORKER_CHECK_MIGRATIONS, createStore, lessonScope, statusRecordSchema, type Binding } from "./store.js"; +import { ACTOR_MIGRATIONS, APPROVAL_MARKER_MIGRATIONS, BASE_MIGRATIONS, CHAIN_MIGRATIONS, GUIDANCE_MIGRATIONS, IDLE_MIGRATIONS, LESSON_COUNTER_MIGRATIONS, LESSON_PR_MIGRATIONS, LESSON_TIER_MIGRATIONS, MIGRATIONS, RATIONALE_MIGRATIONS, VALIDATION_MIGRATIONS, WORKER_CHECK_MIGRATIONS, createStore, lessonScope, statusRecordSchema, type Binding } from "./store.js"; /** MIGRATIONS without the groups appended after the ones these order tests assert. */ -const beforeActorMigrations = MIGRATIONS.slice(0, MIGRATIONS.length - ACTOR_MIGRATIONS.length); +const beforeActorMigrations = MIGRATIONS.slice(0, MIGRATIONS.indexOf(ACTOR_MIGRATIONS[0]!)); function store() { const db = new Database(":memory:"); @@ -296,6 +296,7 @@ describe("lesson citation counts (BB-35 step 2)", () => { for (const sql of VALIDATION_MIGRATIONS) db.exec(sql); for (const sql of GUIDANCE_MIGRATIONS) db.exec(sql); for (const sql of APPROVAL_MARKER_MIGRATIONS) db.exec(sql); + for (const sql of LESSON_PR_MIGRATIONS) db.exec(sql); const s = createStore(db, () => 2000); expect(s.lesson("lsn_old")).toMatchObject({ id: "lsn_old", @@ -338,6 +339,7 @@ describe("the lesson tier migration (BB-35 step 3)", () => { for (const sql of VALIDATION_MIGRATIONS) db.exec(sql); for (const sql of GUIDANCE_MIGRATIONS) db.exec(sql); for (const sql of APPROVAL_MARKER_MIGRATIONS) db.exec(sql); + for (const sql of LESSON_PR_MIGRATIONS) db.exec(sql); return db; }; @@ -395,6 +397,9 @@ describe("the actor rename migration (BB-97)", () => { const legacy = () => { const db = new Database(":memory:"); for (const sql of MIGRATIONS.slice(0, MIGRATIONS.indexOf(ACTOR_MIGRATIONS[0]!))) db.exec(sql); + // The current store prepares its lesson-PR statement, so the column has to + // exist; it is not part of what this test exercises. + for (const sql of LESSON_PR_MIGRATIONS) db.exec(sql); return db; }; const insert = (db: ReturnType, id: string, key: string, status: string | null) => @@ -444,8 +449,9 @@ describe("the actor rename migration (BB-97)", () => { }); it("is appended at the end of MIGRATIONS, so a shipped statement index is never reused", () => { - expect(MIGRATIONS.slice(-ACTOR_MIGRATIONS.length)).toEqual([...ACTOR_MIGRATIONS]); - expect(MIGRATIONS.slice(-(ACTOR_MIGRATIONS.length + APPROVAL_MARKER_MIGRATIONS.length), -ACTOR_MIGRATIONS.length)).toEqual([...APPROVAL_MARKER_MIGRATIONS]); + expect(MIGRATIONS.slice(-(LESSON_PR_MIGRATIONS.length + ACTOR_MIGRATIONS.length), -LESSON_PR_MIGRATIONS.length)).toEqual([...ACTOR_MIGRATIONS]); + expect(MIGRATIONS.slice(-LESSON_PR_MIGRATIONS.length)).toEqual([...LESSON_PR_MIGRATIONS]); + expect(MIGRATIONS.slice(-(LESSON_PR_MIGRATIONS.length + ACTOR_MIGRATIONS.length + APPROVAL_MARKER_MIGRATIONS.length), -(LESSON_PR_MIGRATIONS.length + ACTOR_MIGRATIONS.length))).toEqual([...APPROVAL_MARKER_MIGRATIONS]); }); }); @@ -463,6 +469,7 @@ describe("the rationale migration (BB-35 step 4)", () => { for (const sql of VALIDATION_MIGRATIONS) db.exec(sql); for (const sql of GUIDANCE_MIGRATIONS) db.exec(sql); for (const sql of APPROVAL_MARKER_MIGRATIONS) db.exec(sql); + for (const sql of LESSON_PR_MIGRATIONS) db.exec(sql); return db; }; @@ -509,6 +516,7 @@ describe("the validation migration (BB-35 step 5)", () => { for (const sql of VALIDATION_MIGRATIONS) db.exec(sql); for (const sql of GUIDANCE_MIGRATIONS) db.exec(sql); for (const sql of APPROVAL_MARKER_MIGRATIONS) db.exec(sql); + for (const sql of LESSON_PR_MIGRATIONS) db.exec(sql); return db; }; @@ -559,6 +567,9 @@ describe("the guidance-citation migration (BB-35 step 6)", () => { // Everything before this step: the state of a live database the migration // is applied to. for (const sql of MIGRATIONS.slice(0, MIGRATIONS.indexOf(GUIDANCE_MIGRATIONS[0]!))) db.exec(sql); + // The current store prepares its lesson-PR statement, so the column has to + // exist; it is not part of what this test exercises. + for (const sql of LESSON_PR_MIGRATIONS) db.exec(sql); db.exec(`INSERT INTO bindings (id, project_id, source, entity_id, entity_key, kind, playbook, playbook_version, created_at) VALUES ('bnd_1', 'p1', 'linear', 'uuid-12', 'ENG-12', 'ticket', 'linear-tickets', 'v1', 1000)`); return db; @@ -656,7 +667,7 @@ describe("a worker's pending model check", () => { it("reads workers from before the column existed as not pending", () => { const db = new Database(":memory:"); - for (const sql of [...BASE_MIGRATIONS, ...CHAIN_MIGRATIONS, ...LESSON_COUNTER_MIGRATIONS, ...LESSON_TIER_MIGRATIONS, ...RATIONALE_MIGRATIONS, ...VALIDATION_MIGRATIONS, ...GUIDANCE_MIGRATIONS, ...APPROVAL_MARKER_MIGRATIONS]) db.exec(sql); + for (const sql of [...BASE_MIGRATIONS, ...CHAIN_MIGRATIONS, ...LESSON_COUNTER_MIGRATIONS, ...LESSON_TIER_MIGRATIONS, ...RATIONALE_MIGRATIONS, ...VALIDATION_MIGRATIONS, ...GUIDANCE_MIGRATIONS, ...APPROVAL_MARKER_MIGRATIONS, ...LESSON_PR_MIGRATIONS]) db.exec(sql); db.exec(`INSERT INTO bindings (id, project_id, source, entity_id, entity_key, kind, playbook, playbook_version, created_at) VALUES ('bnd_1', 'p1', 'linear', 'uuid-12', 'ENG-12', 'ticket', 'linear-tickets', 'v1', 1000)`); db.exec(`INSERT INTO workers (thread_id, binding_id, title, status, started_at) VALUES ('thr_old', 'bnd_1', 'ENG-12: Implement', 'running', 1000)`); @@ -699,6 +710,7 @@ describe("the escalation-chain migration (BB-30)", () => { for (const sql of VALIDATION_MIGRATIONS) db.exec(sql); for (const sql of GUIDANCE_MIGRATIONS) db.exec(sql); for (const sql of APPROVAL_MARKER_MIGRATIONS) db.exec(sql); + for (const sql of LESSON_PR_MIGRATIONS) db.exec(sql); let now = 2000; return { db, s: createStore(db, () => now) }; } diff --git a/plugins/stewards/store.ts b/plugins/stewards/store.ts index cfc4286..a479f38 100644 --- a/plugins/stewards/store.ts +++ b/plugins/stewards/store.ts @@ -413,6 +413,13 @@ export const VALIDATION_MIGRATIONS = [ `ALTER TABLE lessons ADD COLUMN validation_filed_at INTEGER`, ]; +// BB-108: a promoted or retired lesson lands as a pull request on the primary +// repo, not a fast-forward. The lesson keeps the pull request's URL so the +// Stewards page can offer to review it, Drop can close it, and the merge is +// observed through the source contract. Additive and defaulted: a lesson from +// before this step has no pull request and behaves as before. +export const LESSON_PR_MIGRATIONS = [`ALTER TABLE lessons ADD COLUMN pr TEXT`, `ALTER TABLE lessons ADD COLUMN retire_pr TEXT`]; + // BB-35 step 6: the hand-written guidance's meter. One row per (project, unit, // binding): the distinct-binding count is the number of rows, and the newest // `last_cited_at` is when it was last cited. The unit's file and text are kept @@ -469,7 +476,7 @@ export const ACTOR_MIGRATIONS = [ WHERE CASE WHEN json_valid(status) THEN json_extract(status, '$.actor') END = 'me'`, ]; -export const MIGRATIONS = [...BASE_MIGRATIONS, ...CHAIN_MIGRATIONS, ...LESSON_COUNTER_MIGRATIONS, ...WORKER_CHECK_MIGRATIONS, ...DECLINED_PR_MIGRATIONS, ...DEPLOYMENT_MIGRATIONS, ...FINISHED_AND_DIGEST_MIGRATIONS, ...LESSON_TIER_MIGRATIONS, ...RATIONALE_MIGRATIONS, ...VALIDATION_MIGRATIONS, ...GUIDANCE_MIGRATIONS, ...IDLE_MIGRATIONS, ...APPROVAL_MARKER_MIGRATIONS, ...ACTOR_MIGRATIONS]; +export const MIGRATIONS = [...BASE_MIGRATIONS, ...CHAIN_MIGRATIONS, ...LESSON_COUNTER_MIGRATIONS, ...WORKER_CHECK_MIGRATIONS, ...DECLINED_PR_MIGRATIONS, ...DEPLOYMENT_MIGRATIONS, ...FINISHED_AND_DIGEST_MIGRATIONS, ...LESSON_TIER_MIGRATIONS, ...RATIONALE_MIGRATIONS, ...VALIDATION_MIGRATIONS, ...GUIDANCE_MIGRATIONS, ...IDLE_MIGRATIONS, ...APPROVAL_MARKER_MIGRATIONS, ...ACTOR_MIGRATIONS, ...LESSON_PR_MIGRATIONS]; // ---- values kept as JSON ------------------------------------------------------------ @@ -977,6 +984,10 @@ export type Lesson = { supersededBy: string | null; /** The `guidance-` branch carrying it, once published. */ branch: string | null; + /** The pull request it lands as (BB-108), or null before one is opened. */ + pr: string | null; + /** The pull request that removes its anchored text (BB-108 retire), or null. */ + retirePr: string | null; /** The canonical `.bb` commit it landed at, once merged. */ mergeSha: string | null; /** The last refusal or explanation from a merge attempt, for the nightly prompt. */ @@ -1214,6 +1225,8 @@ export function createStore(db: Database.Database, now: () => number, options: S const getLesson = db.prepare<[string], Record>("SELECT * FROM lessons WHERE id = ?"); const lessonsFor = db.prepare<[string, number], Record>("SELECT * FROM lessons WHERE project_id = ? AND (? = 1 OR status = 'pending') ORDER BY created_at"); const setLessonBranchStmt = db.prepare("UPDATE lessons SET branch = ?, updated_at = ? WHERE id = ? AND status = 'pending'"); + const setLessonPrStmt = db.prepare("UPDATE lessons SET pr = ?, updated_at = ? WHERE id = ?"); + const setLessonRetirePrStmt = db.prepare("UPDATE lessons SET retire_pr = ?, updated_at = ? WHERE id = ?"); const setLessonNoteStmt = db.prepare("UPDATE lessons SET note = ?, updated_at = ? WHERE id = ?"); const resolveLessonStmt = db.prepare( "UPDATE lessons SET status = ?, merge_sha = COALESCE(?, merge_sha), branch = COALESCE(?, branch), note = ?, updated_at = ?, resolved_at = COALESCE(resolved_at, ?) WHERE id = ?", @@ -1355,6 +1368,8 @@ export function createStore(db: Database.Database, now: () => number, options: S clusterKey: r.cluster_key === null || r.cluster_key === undefined ? null : String(r.cluster_key), supersededBy: r.superseded_by === null || r.superseded_by === undefined ? null : String(r.superseded_by), branch: r.branch === null || r.branch === undefined ? null : String(r.branch), + pr: r.pr === null || r.pr === undefined ? null : String(r.pr), + retirePr: r.retire_pr === null || r.retire_pr === undefined ? null : String(r.retire_pr), mergeSha: r.merge_sha === null || r.merge_sha === undefined ? null : String(r.merge_sha), note: r.note === null || r.note === undefined ? null : String(r.note), hits: Number(r.hits ?? 0), @@ -2148,6 +2163,14 @@ export function createStore(db: Database.Database, now: () => number, options: S setLessonBranch(id: string, branch: string): void { setLessonBranchStmt.run(branch, now(), id); }, + /** Remember the pull request a lesson lands as (BB-108). */ + setLessonPr(id: string, pr: string | null): void { + setLessonPrStmt.run(pr, now(), id); + }, + /** Remember the pull request that removes a lesson's anchored text (BB-108 retire). */ + setLessonRetirePr(id: string, pr: string | null): void { + setLessonRetirePrStmt.run(pr, now(), id); + }, /** Record the last refusal or explanation from a merge attempt, for the prompt. */ noteLesson(id: string, note: string | null): void { setLessonNoteStmt.run(note, now(), id); diff --git a/plugins/stewards/wake.ts b/plugins/stewards/wake.ts index bc1edf1..f95d052 100644 --- a/plugins/stewards/wake.ts +++ b/plugins/stewards/wake.ts @@ -925,7 +925,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, commit, and publish with workspace_publish_guidance; then complete it with steward_update { lessonAction: { id: "", action: "complete" } }. A `boundary` one becomes a Merge/Drop item for the user instead. If a merge 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): 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.', ); 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)}`}`); } @@ -935,7 +935,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 ``, commit, publish with workspace_publish_guidance, then retire it with steward_update { lessonAction: { id: "", action: "retire" } }. 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: a `procedure` one is merged by the project steward, a `boundary` one waits for the owner. The row is kept.', ); for (const l of toRetire) out.push(`- ${l.id} (${l.file}): ${oneLine(l.text)}`); }