From 15b9664ddb87b1d5fb2063dc046ef073fac3f044 Mon Sep 17 00:00:00 2001 From: Dylan Trotter Date: Tue, 10 Nov 2026 05:40:43 +0000 Subject: [PATCH 1/2] stewards: stacked pull requests are visible for their whole life (BB-287) --- plugins/stewards/github.ts | 47 +++++++- plugins/stewards/migrations.snapshot.json | 4 +- plugins/stewards/server.test.ts | 72 +++++++++++- plugins/stewards/server.ts | 106 ++++++++++++++---- .../stewards/skills/build-tickets/SKILL.md | 6 + plugins/stewards/store.test.ts | 18 +-- plugins/stewards/store.ts | 19 +++- plugins/stewards/wake.ts | 11 ++ 8 files changed, 250 insertions(+), 33 deletions(-) diff --git a/plugins/stewards/github.ts b/plugins/stewards/github.ts index a901bb59..565c0aa7 100644 --- a/plugins/stewards/github.ts +++ b/plugins/stewards/github.ts @@ -110,8 +110,23 @@ export interface GithubWriter { * every state, or null when none is visible. A stacked pull request's base * names the branch of the pull request it is stacked on; the plugin reads * this to tell an open base (a merge hold) from a merged one (a retarget). + * `headSha` is the branch's last commit, which a recovery restores a deleted + * base branch at (BB-287). */ - pullByHead(repo: string, head: string): Promise<{ number: number; key: string; state: string; merged: boolean } | null>; + pullByHead(repo: string, head: string): Promise<{ number: number; key: string; state: string; merged: boolean; headSha: string | null } | null>; + /** + * The open pull requests in `repo` whose base branch is `base` (BB-287). A + * binding whose own pull request is the base of one of these must not have + * its branch deleted: the deletion closes the stacked pull request unmerged. + */ + openPullsByBase(repo: string, base: string): Promise>; + /** + * Whether `branch` still exists (BB-287), for a stacked pull request GitHub + * closed because its base branch was deleted (`delete_branch_on_merge`). A + * read failure is reported as existing, so the recovery never fires on a + * transient error. + */ + branchExists(repo: string, branch: 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). */ @@ -203,13 +218,39 @@ export function createGhWriter(token: TokenReader = createGhTokenReader(), doFet if (!Array.isArray(body)) return null; for (const item of body) { if (item === null || typeof item !== "object") continue; - const pr = item as { number?: unknown; state?: unknown; merged_at?: unknown; head?: { ref?: unknown } }; + const pr = item as { number?: unknown; state?: unknown; merged_at?: unknown; head?: { ref?: unknown; sha?: unknown } }; if (typeof pr.number !== "number") continue; if (typeof pr.head?.ref === "string" && pr.head.ref !== head) continue; - return { number: pr.number, key: `${repo}#${pr.number}`, state: typeof pr.state === "string" ? pr.state : "", merged: pr.merged_at !== null && pr.merged_at !== undefined }; + return { number: pr.number, key: `${repo}#${pr.number}`, state: typeof pr.state === "string" ? pr.state : "", merged: pr.merged_at !== null && pr.merged_at !== undefined, headSha: typeof pr.head?.sha === "string" && pr.head.sha !== "" ? pr.head.sha : null }; } return null; }, + async openPullsByBase(repo, base) { + // `base` filters by the base branch's name; `state=open` is what makes + // this the stacked-on-me check, and the list is newest first. + const query = `base=${encodeURIComponent(base)}&state=open&sort=created&direction=desc&per_page=100`; + const response = await request("GET", `/repos/${repo}/pulls?${query}`, undefined); + const body = (await response.json().catch(() => null)) as unknown; + if (!Array.isArray(body)) return []; + const pulls: Array<{ number: number; key: string; state: string }> = []; + for (const item of body) { + if (item === null || typeof item !== "object") continue; + const pr = item as { number?: unknown; state?: unknown; base?: { ref?: unknown } }; + if (typeof pr.number !== "number") continue; + if (typeof pr.base?.ref === "string" && pr.base.ref !== base) continue; + pulls.push({ number: pr.number, key: `${repo}#${pr.number}`, state: typeof pr.state === "string" ? pr.state : "" }); + } + return pulls; + }, + async branchExists(repo, branch) { + try { + await request("GET", `/repos/${repo}/branches/${encodeURIComponent(branch)}`, undefined); + return true; + } catch (error) { + if (error instanceof GithubApiError && / 404 /u.test(error.message)) return false; + throw error; + } + }, async dispatchWorkflow(repo, workflow, ref, inputs) { await request("POST", `/repos/${repo}/actions/workflows/${encodeURIComponent(workflow)}/dispatches`, { ref, inputs }); }, diff --git a/plugins/stewards/migrations.snapshot.json b/plugins/stewards/migrations.snapshot.json index 71187085..ea94ecc7 100644 --- a/plugins/stewards/migrations.snapshot.json +++ b/plugins/stewards/migrations.snapshot.json @@ -116,5 +116,7 @@ "dab0f2a29dd1b4d8d65a58fb712c4d6689105d6c5bbe90e95dc3accff924c3f1", "699df5c091ae8f12adf16a45faad54c866cd28c58963b3a9132ab6708617d35b", "0cb5723b6d354beb4c2e6e60fabb5b5e3ddf0245bc755b3b4bcd3cbce6b6fd56", - "a8f6be062abc53e51ea5b4acd27627cf915242aa1b0a60e7e32dc3184ab65b13" + "a8f6be062abc53e51ea5b4acd27627cf915242aa1b0a60e7e32dc3184ab65b13", + "cc5d181530cedb4b86b6a61a9cee4f19d141024e812a2e7b944c7796bb211b47", + "bc78b2041e26800eb88cc1de2043140d376b015e92af6105b18c84c5735b48a5" ] diff --git a/plugins/stewards/server.test.ts b/plugins/stewards/server.test.ts index b68445b6..a4f435c5 100644 --- a/plugins/stewards/server.test.ts +++ b/plugins/stewards/server.test.ts @@ -110,6 +110,8 @@ function world() { 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(); + /** Branches the test registered as existing, for the BB-287 recovery read. */ + const githubBranches = new Set(); /** The survey's GitHub reads: a recursive tree, branch protection, labels and file text. */ const githubTrees = new Map>(); const githubBranchProtection = new Map(); @@ -189,10 +191,28 @@ function world() { if (e.unit.toLowerCase() !== repo.toLowerCase()) continue; const number = Number(/(\d+)$/u.exec(e.key)?.[1] ?? ""); if (!Number.isInteger(number) || number <= 0) continue; - return { number, key: e.key, state: e.state, merged: e.pull.merged === true || e.state.toLowerCase() === "merged" }; + return { number, key: e.key, state: e.state, merged: e.pull.merged === true || e.state.toLowerCase() === "merged", headSha: e.pull.headSha }; } return null; }, + // BB-287: the open pull requests whose base is `base`, the stacked-on-me + // check. The fake scans the entities the test registered. + openPullsByBase: async (repo: string, base: string) => { + const pulls: Array<{ number: number; key: string; state: string }> = []; + for (const e of entities.values()) { + if (e.pull === null || e.pull.base !== base) continue; + if (e.unit.toLowerCase() !== repo.toLowerCase()) continue; + if (e.pull.merged === true || e.state.toLowerCase() === "merged" || e.state.toLowerCase() === "closed") continue; + const number = Number(/(\d+)$/u.exec(e.key)?.[1] ?? ""); + if (!Number.isInteger(number) || number <= 0) continue; + pulls.push({ number, key: e.key, state: e.state }); + } + return pulls; + }, + // BB-287: a branch the test registered as existing; a base branch absent + // from the set is gone, which fires the recovery wake for a closed stacked + // pull request. + branchExists: async (repo: string, branch: string) => githubBranches.has(`${repo.toLowerCase()} ${branch}`), dispatchWorkflow: async (repo: string, workflow: string, ref: string, inputs: Record) => { if (mergeDispatchError !== null) throw new Error(mergeDispatchError); githubDispatches.push({ repo, workflow, ref, inputs }); @@ -523,7 +543,7 @@ function world() { }, }; let now = Date.parse("2026-10-14T15:00:00Z"); - return { files, writes, writtenContent, fileContents, repos, primary, entities, projectList, dependabotAlerts, sourceDeployments, sourceFiles, sourceCommits, sourceRuns, sourceCompares, labelEnsureCalls, labelsEnsured, labelEnsureFailures, setLabelEnsureUnavailable: (value: boolean) => { labelEnsureUnavailable = value; }, setPushUnavailable: (value: boolean) => { pushUnavailable = value; }, setLabelEnsureThrow: (value: string | null) => { labelEnsureThrow = value; }, setLayout: (value: "primary" | "directory") => { layout = value; primary = value === "directory" ? null : { dir: "bb", url: "https://github.com/acme/bb.git" }; }, viewers, setLinearMine: (teams: Array<{ unit: string; name: string }>) => { linearMine = teams; }, setMissingStates: (source: "linear" | "github", unit: string, states: string[]) => { missingStates.set(`${source}:${unit.toLowerCase()}`, states); }, setMissingLabel: (source: "linear" | "github", unit: string, labels: string[]) => { missingLabels.set(`${source}:${unit.toLowerCase()}`, labels); }, rpcCalls, githubTrees, githubBranchProtection, githubLabels, setSnapshotError: (id: string, fail: boolean) => { if (fail) snapshotErrors.add(id); else snapshotErrors.delete(id); }, setSearchUnsupported: (value: boolean) => { searchUnsupported = value; }, setSearchError: (value: string | null) => { searchError = value; }, setCreatePullError: (value: string | null) => { createPullError = value; }, setMergePullError: (value: string | null) => { mergePullError = value; }, setMoveHeadOnMerge: (value: boolean) => { moveHeadOnMerge = value; }, setPullDetailsError: (value: string | null) => { pullDetailsError = value; }, setRetryConflict: (value: boolean) => { retryConflict = value; }, setRetryQueued: (value: boolean) => { retryQueued = value; }, threads, deletes, sections, usage, metadata, spawns, sends, retries, retryEvents, queuedDeletes, userTurns, githubComments, githubReviews, githubMerges, githubPulls, githubClosedPulls, githubDefaultBranches, githubFiles, githubFileContents, githubDispatches, setMergeDispatchError: (value: string | null) => { mergeDispatchError = value; }, github, sdk, uiPreferences, uiPreferencesWrites, now: () => now, advance: (ms: number) => (now += ms) }; + return { files, writes, writtenContent, fileContents, repos, primary, entities, projectList, dependabotAlerts, sourceDeployments, sourceFiles, sourceCommits, sourceRuns, sourceCompares, labelEnsureCalls, labelsEnsured, labelEnsureFailures, setLabelEnsureUnavailable: (value: boolean) => { labelEnsureUnavailable = value; }, setPushUnavailable: (value: boolean) => { pushUnavailable = value; }, setLabelEnsureThrow: (value: string | null) => { labelEnsureThrow = value; }, setLayout: (value: "primary" | "directory") => { layout = value; primary = value === "directory" ? null : { dir: "bb", url: "https://github.com/acme/bb.git" }; }, viewers, setLinearMine: (teams: Array<{ unit: string; name: string }>) => { linearMine = teams; }, setMissingStates: (source: "linear" | "github", unit: string, states: string[]) => { missingStates.set(`${source}:${unit.toLowerCase()}`, states); }, setMissingLabel: (source: "linear" | "github", unit: string, labels: string[]) => { missingLabels.set(`${source}:${unit.toLowerCase()}`, labels); }, rpcCalls, githubTrees, githubBranchProtection, githubLabels, setSnapshotError: (id: string, fail: boolean) => { if (fail) snapshotErrors.add(id); else snapshotErrors.delete(id); }, setSearchUnsupported: (value: boolean) => { searchUnsupported = value; }, setSearchError: (value: string | null) => { searchError = value; }, setCreatePullError: (value: string | null) => { createPullError = value; }, setMergePullError: (value: string | null) => { mergePullError = value; }, setMoveHeadOnMerge: (value: boolean) => { moveHeadOnMerge = value; }, setPullDetailsError: (value: string | null) => { pullDetailsError = value; }, setRetryConflict: (value: boolean) => { retryConflict = value; }, setRetryQueued: (value: boolean) => { retryQueued = value; }, threads, deletes, sections, usage, metadata, spawns, sends, retries, retryEvents, queuedDeletes, userTurns, githubComments, githubReviews, githubMerges, githubPulls, githubClosedPulls, githubDefaultBranches, githubBranches, githubFiles, githubFileContents, githubDispatches, setMergeDispatchError: (value: string | null) => { mergeDispatchError = value; }, github, sdk, uiPreferences, uiPreferencesWrites, now: () => now, advance: (ms: number) => (now += ms) }; } async function setup(extra: Partial = {}, settings: Record = {}, seedProvider?: string | null | "fail-list" | "fail-set", seedNavigation?: string | null | "fail-list" | "fail-set") { @@ -7418,6 +7438,54 @@ describe("needs you items", () => { expect(t.hooks.store.openNeeds(b.id).some((n) => n.kind === "merge")).toBe(false); }); + it("wakes the steward to retarget a stacked pull request whose base merged before its gate is met (BB-287)", async () => { + const t = await withPull([]); + // No reviewer marker: the gate is not met. The base pull request merges + // anyway, and the retarget wake must fire all the same (BB-287). + t.entities.set("acme/web#8", pull({ id: "acme/web#8", key: "acme/web#8", url: "https://github.com/acme/web/pull/8", title: "Base", state: "merged" }, { merged: true, mergeCommit: "b".repeat(40), head: "bb/bb-256-parent", base: "main" })); + t.entities.set("acme/web#7", pull({}, { base: "bb/bb-256-parent" })); + await t.ring("linear", "uuid-12"); + await t.flush(); + expect(t.githubMerges).toEqual([]); + expect(t.hooks.store.find("ENG-12")!.retargetPulls).toEqual(["acme/web#8"]); + expect(t.sends.filter((s) => s.threadId === t.steward).some((s) => /Retarget acme\/web#7: its base acme\/web#8 has merged/u.test(s.text))).toBe(true); + }); + + it("tells a binding its pull request is the base of an open pull request, in the wake and in show (BB-287)", async () => { + const t = await withPull([]); + // #9 is stacked on #7's branch. The binding must know not to delete it. + t.entities.set("acme/web#9", pull({ id: "acme/web#9", key: "acme/web#9", url: "https://github.com/acme/web/pull/9", title: "Stacked" }, { head: "bb/eng-12-stacked", base: "bb/eng-12-retry" })); + await t.ring("linear", "uuid-12"); + await t.flush(); + expect(t.hooks.store.find("ENG-12")!.stackedPulls).toEqual(["acme/web#9"]); + expect(t.sends.filter((s) => s.threadId === t.steward).some((s) => /Base of open pull request: acme\/web#9 is stacked on acme\/web#7/u.test(s.text))).toBe(true); + // The standing section of the wake names it too, so every later wake carries it. + const wake = t.sends.filter((s) => s.threadId === t.steward).at(-1)!.text; + expect(wake).toMatch(/BASE OF: acme\/web#9/u); + // `bb stewards show` renders the same fact. + const shown = (await t.harness.behavior.runCli(["show", "ENG-12", "--project", "p1"])).stdout; + expect(shown).toContain("BASE OF: acme/web#9"); + }); + + it("wakes the steward to restore the base branch when a stacked pull request closes unmerged with its base branch gone (BB-287)", async () => { + const t = await withPull([]); + // The base pull request merged and its branch was deleted; GitHub closed + // the stacked pull request unmerged. The wake names the sha to restore the + // branch at and says the approvals carry. + t.entities.set("acme/web#8", pull({ id: "acme/web#8", key: "acme/web#8", url: "https://github.com/acme/web/pull/8", title: "Base", state: "merged" }, { merged: true, mergeCommit: "b".repeat(40), head: "bb/bb-256-parent", base: "main", headSha: "deadbeef1" })); + t.entities.set("acme/web#7", pull({ state: "closed" }, { merged: false, base: "bb/bb-256-parent" })); + await t.ring("linear", "uuid-12"); + await t.flush(); + const wake = t.sends.filter((s) => s.threadId === t.steward).at(-1)!.text; + expect(wake).toMatch(/restore the branch at .*deadbeef1.*reopen or replace.*approvals carry/u); + expect(t.hooks.store.find("ENG-12")!.stackedRecovered).toEqual(["acme/web#7"]); + // The recovery wake is once per pull request, not on every look. + const wakes = t.sends.filter((s) => s.threadId === t.steward).length; + await t.ring("linear", "uuid-12"); + await t.flush(); + expect(t.sends.filter((s) => s.threadId === t.steward).length).toBe(wakes); + }); + it("does not merge while a worker is still running", async () => { const t = await setup(); t.entities.set("uuid-12", issue({ linkedPulls: [{ key: "acme/web#7", url: "https://github.com/acme/web/pull/7" }] })); diff --git a/plugins/stewards/server.ts b/plugins/stewards/server.ts index e4102669..83a9cd57 100644 --- a/plugins/stewards/server.ts +++ b/plugins/stewards/server.ts @@ -6904,6 +6904,12 @@ export function createStewardsPlugin(deps: StewardsDeps): (bb: BbPluginApi) => P // is judged on this set, never on the one the loop mutates, so the look // that first sends the wake cannot also raise the stall. const retargetSentBefore = new Set(retargetBefore); + // BB-287: the open pull requests stacked on this binding's own pull + // requests, and the stacked pull requests already woken about after + // closing with their base branch gone. + const stackedOnMe: string[] = []; + const recoveredPulls = [...b.stackedRecovered]; + const recoveredBefore = new Set(recoveredPulls.map((k) => lc(k))); for (const pull of observedPulls(b, observed)) { const url = pull.url ?? pull.key; if (ownPulls !== null && !pullBelongsToTicket(b, pull, ownPulls)) { @@ -6921,10 +6927,7 @@ export function createStewardsPlugin(deps: StewardsDeps): (bb: BbPluginApi) => P if (hadApprove && !closed && (g.ownerApproved || g.projectApproved) && !store.openNeeds(b.id).some((n) => n.kind === "approve-pr" && n.url === url)) { events.push({ kind: "approved", pull: pull.key }); } - if (closed) { - store.closeNeedByKey(b.id, "merge", url, "The pull request is closed."); - continue; - } + const ref = parsePullRef(pull.url ?? pull.key) ?? parsePullRef(url); // BB-170: on a teammate's pull request (a review steward), the owner's // approval is their last step and the author merges, so the plugin // files no Merge item. A review binding is a teammate's pull request by @@ -6937,6 +6940,50 @@ export function createStewardsPlugin(deps: StewardsDeps): (bb: BbPluginApi) => P !dependencyBot && !playbook.ownerMerges && (b.kind === "review" || (b.kind === "pr" && pull.author !== null && ownerLogin !== null && lc(pull.author) !== lc(ownerLogin))); + if (closed) { + store.closeNeedByKey(b.id, "merge", url, "The pull request is closed."); + // BB-287: a stacked pull request GitHub closed because its base + // branch was deleted gets a recovery wake, once per pull request. + if (!foreignOwner && pull.pull?.merged !== true && lc(pull.state) === "closed" && !recoveredBefore.has(lc(pull.key))) { + const recovery = await stackedCloseRecovery(pull, ref); + if (recovery !== null) { + changes.push({ text: recovery, actionable: true }); + recoveredPulls.push(pull.key); + recoveredBefore.add(lc(pull.key)); + } + } + continue; + } + // BB-287: what is stacked on this open pull request. The binding whose + // pull request is the base of another open one is told, so its worker + // does not delete the branch and close the stacked pull request. + if (!foreignOwner && ref !== null && typeof pull.pull?.head === "string" && pull.pull.head !== "") { + const stacked = await github.openPullsByBase(ref.repo, pull.pull.head).catch(() => []); + for (const s of stacked) { + if (lc(s.key) === lc(pull.key)) continue; + stackedOnMe.push(s.key); + if (!b.stackedPulls.some((x) => lc(x) === lc(s.key))) { + changes.push({ text: `Base of open pull request: ${s.key} is stacked on ${pull.key}. Do not delete ${pull.key}'s branch until ${s.key} is retargeted or closed: before deleting any branch run \`gh pr list --base --state open\` and leave a branch that is the base of an open pull request.`, actionable: true }); + } + } + } + // BB-287: the stacked-PR disposition is read for every open own pull + // request, not only a gate-met one, so a base merge (and the retarget + // it needs) reaches the steward whatever the gate state. + const base = bindingMayMerge(b, playbook) && ref !== null && !foreignOwner ? await pullBaseDisposition(ref, pull) : null; + const openBase = base !== null && base.kind === "open" ? base : null; + const mergedBase = base !== null && base.kind === "merged" ? base : null; + if (openBase !== null) derivedMergeBehind.push(openBase.key); + // BB-264/BB-287: the stacked-PR wakes fire whatever the gate state. + if (openBase !== null && !heldBefore.has(lc(openBase.key))) { + changes.push({ text: `Merge held: ${pull.key} is stacked on ${openBase.key}, which is still open. The plugin files no Merge item; it merges once the base lands and the pull request is retargeted to the default branch.`, actionable: true }); + } + if (mergedBase !== null && !retargetBefore.has(lc(mergedBase.key))) { + const prRef = ref === null ? pull.key : `${ref.repo}#${ref.number}`; + changes.push({ text: `Retarget ${pull.key}: its base ${mergedBase.key} has merged. Rebase it onto the default branch (\`git fetch origin && git rebase --onto origin/ \`), push, then retarget it (\`gh pr edit ${prRef} --base \`); the plugin merges it once the default branch is its base and the checks on the new head are green.`, actionable: true }); + retargetPulls.push(mergedBase.key); + retargetBefore.add(lc(mergedBase.key)); + } if (g.gateMet) { // BB-285: whether this look merged the pull request, so the // gate-met-with-nobody-acting check below does not race its own merge. @@ -6948,11 +6995,6 @@ export function createStewardsPlugin(deps: StewardsDeps): (bb: BbPluginApi) => P // steward-filed item (a refused merge) is always left alone. // BB-264: an open base pull request is the same hold, derived from // the pull request's base rather than reported. - const ref = parsePullRef(pull.url ?? pull.key) ?? parsePullRef(url); - const base = bindingMayMerge(b, playbook) && ref !== null && !foreignOwner ? await pullBaseDisposition(ref, pull) : null; - const openBase = base !== null && base.kind === "open" ? base : null; - const mergedBase = base !== null && base.kind === "merged" ? base : null; - if (openBase !== null) derivedMergeBehind.push(openBase.key); const mergeHeld = userMergeBehind.length > 0 || openBase !== null; if (foreignOwner) { const open = store.openNeeds(b.id).find((n) => n.kind === "merge" && n.url === url && n.filedBy === "plugin"); @@ -6964,9 +7006,6 @@ export function createStewardsPlugin(deps: StewardsDeps): (bb: BbPluginApi) => P const held = [...userMergeBehind, ...(openBase === null ? [] : [openBase.key])]; store.closeNeed(open.id, `held behind ${(keys.length > 0 ? keys : held).join(", ")}`); } - if (openBase !== null && !heldBefore.has(lc(openBase.key))) { - changes.push({ text: `Merge held: ${pull.key} is stacked on ${openBase.key}, which is still open. The plugin files no Merge item; it merges once the base lands and the pull request is retargeted to the default branch.`, actionable: true }); - } } else if (mergedBase !== null) { // BB-264: GitHub does not retarget a stacked pull request when its // base merges (the branch survives), so the base is the head of a @@ -6977,12 +7016,6 @@ export function createStewardsPlugin(deps: StewardsDeps): (bb: BbPluginApi) => P // old one are still green. const open = store.openNeeds(b.id).find((n) => n.kind === "merge" && n.url === url && n.filedBy === "plugin"); if (open !== undefined) store.closeNeed(open.id, `The base ${mergedBase.key} has merged; retarget to the default branch.`); - if (!retargetBefore.has(lc(mergedBase.key))) { - const prRef = ref === null ? pull.key : `${ref.repo}#${ref.number}`; - changes.push({ text: `Retarget ${pull.key}: its base ${mergedBase.key} has merged. Rebase it onto the default branch (\`git fetch origin && git rebase --onto origin/ \`), push, then retarget it (\`gh pr edit ${prRef} --base \`); the plugin merges it once the default branch is its base and the checks on the new head are green.`, actionable: true }); - retargetPulls.push(mergedBase.key); - retargetBefore.add(lc(mergedBase.key)); - } // BB-285: a retarget wake nobody answered. Once the wake has been // sent (recorded in retargetPulls) and the pull request is still // based on the merged branch after the window, escalate `stuck`. @@ -7087,6 +7120,17 @@ export function createStewardsPlugin(deps: StewardsDeps): (bb: BbPluginApi) => P if (nextRetargetPulls.length !== b.retargetPulls.length || nextRetargetPulls.some((k, i) => k !== b.retargetPulls[i])) { b = store.patch(b.id, { retargetPulls: nextRetargetPulls }); } + // BB-287: what is stacked on this binding's pull requests, so `bb + // stewards show` and the sidebar carry it; and the stacked pull requests + // already given a recovery wake. + const nextStackedPulls = [...new Set(stackedOnMe)].slice(-20); + if (nextStackedPulls.length !== b.stackedPulls.length || nextStackedPulls.some((k, i) => k !== b.stackedPulls[i])) { + b = store.patch(b.id, { stackedPulls: nextStackedPulls }); + } + const nextRecoveredPulls = recoveredPulls.slice(-20); + if (nextRecoveredPulls.length !== b.stackedRecovered.length || nextRecoveredPulls.some((k, i) => k !== b.stackedRecovered[i])) { + b = store.patch(b.id, { stackedRecovered: nextRecoveredPulls }); + } if (approvalRaised) await wakeProject(b.projectId, `an owner-reviewed pull request from ${b.entityKey}`, { digestDue: false }); // BB-231: a gate item filed this look may cover an open route-`user` // escalation or question; ask the project steward once, per new item. @@ -8011,6 +8055,30 @@ export function createStewardsPlugin(deps: StewardsDeps): (bb: BbPluginApi) => P return { kind: "other" }; } + /** + * BB-287: the recovery wake for a stacked pull request GitHub closed + * because its base branch was deleted (`delete_branch_on_merge`, or a hand + * deletion). Null when the pull request is not closed unmerged, is not + * stacked, or its base branch still exists. The text names the base branch + * and the sha to restore it at, and says the approvals carry. A read that + * fails is treated as the branch existing, so a transient error never + * fires the recovery. + */ + async function stackedCloseRecovery(pull: EntitySnapshot, ref: { repo: string; number: number } | null): Promise { + if (ref === null) return null; + const base = pull.pull?.base ?? null; + if (base === null || base === "") return null; + const def = await github.defaultBranch(ref.repo).catch(() => null); + if (def !== null && def === base) return null; + const exists = await github.branchExists(ref.repo, base).catch(() => true); + if (exists) return null; + const basePull = await github.pullByHead(ref.repo, base).catch(() => null); + const sha = basePull?.headSha ?? null; + const at = sha === null ? "the base branch's last commit" : `\`${sha}\``; + const command = sha === null ? `git push origin :refs/heads/${base}` : `git push origin ${sha}:refs/heads/${base}`; + return `Stacked pull request closed: restore the branch at ${at}, reopen or replace, approvals carry. ${pull.key} closed unmerged and its base branch ${base} is gone; restore it with \`${command}\`, then reopen ${pull.key} (or open a replacement) and retarget it to the default branch.`; + } + /** * The key a refused merge is recorded against (BB-257): the head sha and * the reviewer marker and lane that made the gate met. A refusal under the @@ -10533,7 +10601,7 @@ export function createStewardsPlugin(deps: StewardsDeps): (bb: BbPluginApi) => P ? `cannot read the target: ${sentry.error}` : `looked ${iso(sentry.at)}: ${sentry.findings.length === 0 ? "nothing new" : `${sentry.findings.length} issue(s) new, regressed or rung`}`; const needs = store.openNeeds(b.id); - const flags = [b.endedAt === null ? null : `ended: ${b.endReason}`, watch, b.adoptedBy === null ? null : `adopted by ${b.adoptedBy}`, b.priority === null ? null : `priority ${b.priority}`, b.noMorePulls === true ? "no more pull requests" : null, b.hold === null ? null : `HOLD: ${b.hold}${b.holdBehind.length === 0 ? "" : ` (behind ${b.holdBehind.join(", ")})`}${b.holdStack.length === 0 ? "" : ` (stacked on ${b.holdStack.join(", ")})`}`, b.mergeBehind.length === 0 ? null : `MERGE HOLD: behind ${b.mergeBehind.join(", ")}`, ...open.map((x) => `${x.kind} escalation ${x.id} with ${x.route}`), ...needs.map((n) => `${n.kind} need ${n.id}${n.url === "" ? "" : ` (${n.url})`}`), unheardWait(s, open, needs), asksForOpenApproval(s, open, needs)].filter((f) => f !== null); + const flags = [b.endedAt === null ? null : `ended: ${b.endReason}`, watch, b.adoptedBy === null ? null : `adopted by ${b.adoptedBy}`, b.priority === null ? null : `priority ${b.priority}`, b.noMorePulls === true ? "no more pull requests" : null, b.hold === null ? null : `HOLD: ${b.hold}${b.holdBehind.length === 0 ? "" : ` (behind ${b.holdBehind.join(", ")})`}${b.holdStack.length === 0 ? "" : ` (stacked on ${b.holdStack.join(", ")})`}`, b.mergeBehind.length === 0 ? null : `MERGE HOLD: behind ${b.mergeBehind.join(", ")}`, b.stackedPulls.length === 0 ? null : `BASE OF: ${b.stackedPulls.join(", ")}`, ...open.map((x) => `${x.kind} escalation ${x.id} with ${x.route}`), ...needs.map((n) => `${n.kind} need ${n.id}${n.url === "" ? "" : ` (${n.url})`}`), unheardWait(s, open, needs), asksForOpenApproval(s, open, needs)].filter((f) => f !== null); return `${b.entityKey} — ${b.kind}, ${b.phase ?? "?"}${roles.length === 0 ? "" : `, role ${roles.join(", ")}`}${b.threadId === null ? "" : `, thread ${b.threadId}`}\n ${s === null ? "no report yet" : `${s.summary} Next: ${s.nextAction} (${s.actor})${b.noWakeSource ? "; waiting, no wake source" : s.waitingOn === null ? "" : `; waiting on ${s.waitingOn}${b.requeueUntil === null ? "" : ` until ${iso(b.requeueUntil).slice(0, 10)}`}`}`}${flags.length > 0 ? `\n [${flags.join("; ")}]` : ""}`; }) .join("\n"); diff --git a/plugins/stewards/skills/build-tickets/SKILL.md b/plugins/stewards/skills/build-tickets/SKILL.md index 61c58139..a7968c9f 100644 --- a/plugins/stewards/skills/build-tickets/SKILL.md +++ b/plugins/stewards/skills/build-tickets/SKILL.md @@ -114,6 +114,12 @@ the default branch: - If the base pull request closes without merging, retarget yours to the default branch (`gh pr edit --base `, rebasing or merging the default branch as needed) and carry on as an ordinary pull request. +- **Never delete a branch that is the base of an open pull request.** A base + branch deleted by `delete_branch_on_merge` or by hand closes the stacked + pull request unmerged. Before deleting any branch, run + `gh pr list --base --state open`; if it lists an open pull request, + leave the branch. The steward's wake says when another open pull request is + stacked on yours. ## Test diff --git a/plugins/stewards/store.test.ts b/plugins/stewards/store.test.ts index f6bab618..60bdfa4b 100644 --- a/plugins/stewards/store.test.ts +++ b/plugins/stewards/store.test.ts @@ -1,7 +1,7 @@ import Database from "better-sqlite3"; import { describe, expect, it } from "vitest"; import { z } from "zod"; -import { ACTOR_MIGRATIONS, APPROVAL_MARKER_MIGRATIONS, ASSUMPTION_MIGRATIONS, BASE_MIGRATIONS, CHAIN_MIGRATIONS, DATED_WAIT_MIGRATIONS, DEPS_DECISION_MIGRATIONS, ESCALATION_ROUTE_MIGRATIONS, GUIDANCE_MIGRATIONS, IDLE_MIGRATIONS, LESSON_COUNTER_MIGRATIONS, LESSON_PR_MIGRATIONS, LESSON_TIER_MIGRATIONS, MERGE_BEHIND_MIGRATIONS, MIGRATIONS, PULL_WATCH_MIGRATIONS, RATIONALE_MIGRATIONS, RETARGET_PULLS_MIGRATIONS, REVIEWER_MODEL_MIGRATIONS, SERVER_SETTINGS_MIGRATIONS, STACK_MIGRATIONS, VALIDATION_MIGRATIONS, WAKE_RETRY_MIGRATIONS, WAKE_SOURCE_MIGRATIONS, WORKER_CHECK_MIGRATIONS, WORKER_STOP_REASON_MIGRATIONS, createStore, lessonScope, statusRecordSchema, type Binding } from "./store.js"; +import { ACTOR_MIGRATIONS, APPROVAL_MARKER_MIGRATIONS, ASSUMPTION_MIGRATIONS, BASE_MIGRATIONS, CHAIN_MIGRATIONS, DATED_WAIT_MIGRATIONS, DEPS_DECISION_MIGRATIONS, ESCALATION_ROUTE_MIGRATIONS, GUIDANCE_MIGRATIONS, IDLE_MIGRATIONS, LESSON_COUNTER_MIGRATIONS, LESSON_PR_MIGRATIONS, LESSON_TIER_MIGRATIONS, MERGE_BEHIND_MIGRATIONS, MIGRATIONS, PULL_WATCH_MIGRATIONS, RATIONALE_MIGRATIONS, RETARGET_PULLS_MIGRATIONS, REVIEWER_MODEL_MIGRATIONS, SERVER_SETTINGS_MIGRATIONS, STACK_MIGRATIONS, STACKED_PULLS_MIGRATIONS, VALIDATION_MIGRATIONS, WAKE_RETRY_MIGRATIONS, WAKE_SOURCE_MIGRATIONS, WORKER_CHECK_MIGRATIONS, WORKER_STOP_REASON_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.indexOf(ACTOR_MIGRATIONS[0]!)); @@ -486,8 +486,9 @@ describe("the actor rename migration (BB-97)", () => { // and BB-215's stack group follows it (migrations.test.ts pins the whole // list, this only checks the group order). // BB-256 appended the merge-hold group after BB-255's wake-retry group, - // and BB-264 appended the retarget-pulls column after that. - const groups = [RETARGET_PULLS_MIGRATIONS, MERGE_BEHIND_MIGRATIONS, WAKE_RETRY_MIGRATIONS, DATED_WAIT_MIGRATIONS, ASSUMPTION_MIGRATIONS, WORKER_STOP_REASON_MIGRATIONS, DEPS_DECISION_MIGRATIONS, ESCALATION_ROUTE_MIGRATIONS, STACK_MIGRATIONS, PULL_WATCH_MIGRATIONS, SERVER_SETTINGS_MIGRATIONS, WAKE_SOURCE_MIGRATIONS, REVIEWER_MODEL_MIGRATIONS, LESSON_PR_MIGRATIONS, ACTOR_MIGRATIONS, APPROVAL_MARKER_MIGRATIONS]; + // BB-264 appended the retarget-pulls column after that, and BB-287 + // appended the stacked-pull columns after that. + const groups = [STACKED_PULLS_MIGRATIONS, RETARGET_PULLS_MIGRATIONS, MERGE_BEHIND_MIGRATIONS, WAKE_RETRY_MIGRATIONS, DATED_WAIT_MIGRATIONS, ASSUMPTION_MIGRATIONS, WORKER_STOP_REASON_MIGRATIONS, DEPS_DECISION_MIGRATIONS, ESCALATION_ROUTE_MIGRATIONS, STACK_MIGRATIONS, PULL_WATCH_MIGRATIONS, SERVER_SETTINGS_MIGRATIONS, WAKE_SOURCE_MIGRATIONS, REVIEWER_MODEL_MIGRATIONS, LESSON_PR_MIGRATIONS, ACTOR_MIGRATIONS, APPROVAL_MARKER_MIGRATIONS]; let end = MIGRATIONS.length; for (const group of groups) { expect(MIGRATIONS.slice(end - group.length, end)).toEqual([...group]); @@ -575,10 +576,13 @@ describe("the escalation route rename migration (BB-209)", () => { // provenance column) was appended after this one, BB-242's // worker-stop-reason column after that, BB-251's assumptions table after // that, BB-247's dated-wait columns after that, BB-255's wake-retry - // columns after that, BB-256's merge-hold column after that, and BB-264's - // retarget-pulls column after that. - const withRetarget = MIGRATIONS.slice(0, -RETARGET_PULLS_MIGRATIONS.length); - expect(MIGRATIONS.slice(-RETARGET_PULLS_MIGRATIONS.length)).toEqual([...RETARGET_PULLS_MIGRATIONS]); + // columns after that, BB-256's merge-hold column after that, BB-264's + // retarget-pulls column after that, and BB-287's stacked-pull columns + // after that. + const withStacked = MIGRATIONS.slice(0, -STACKED_PULLS_MIGRATIONS.length); + expect(MIGRATIONS.slice(-STACKED_PULLS_MIGRATIONS.length)).toEqual([...STACKED_PULLS_MIGRATIONS]); + const withRetarget = withStacked.slice(0, -RETARGET_PULLS_MIGRATIONS.length); + expect(withStacked.slice(-RETARGET_PULLS_MIGRATIONS.length)).toEqual([...RETARGET_PULLS_MIGRATIONS]); const withMergeBehind = withRetarget.slice(0, -MERGE_BEHIND_MIGRATIONS.length); expect(withRetarget.slice(-MERGE_BEHIND_MIGRATIONS.length)).toEqual([...MERGE_BEHIND_MIGRATIONS]); const withWakeRetry = withMergeBehind.slice(0, -WAKE_RETRY_MIGRATIONS.length); diff --git a/plugins/stewards/store.ts b/plugins/stewards/store.ts index e11a9f7f..754740ec 100644 --- a/plugins/stewards/store.ts +++ b/plugins/stewards/store.ts @@ -625,7 +625,15 @@ export const MERGE_BEHIND_MIGRATIONS = [`ALTER TABLE bindings ADD COLUMN merge_b // Additive and defaulted: an old row reads as nothing woken yet. export const RETARGET_PULLS_MIGRATIONS = [`ALTER TABLE bindings ADD COLUMN retarget_pulls TEXT NOT NULL DEFAULT '[]'`]; -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, ...REVIEWER_MODEL_MIGRATIONS, ...WAKE_SOURCE_MIGRATIONS, ...SERVER_SETTINGS_MIGRATIONS, ...PULL_WATCH_MIGRATIONS, ...STACK_MIGRATIONS, ...ESCALATION_ROUTE_MIGRATIONS, ...DEPS_DECISION_MIGRATIONS, ...WORKER_STOP_REASON_MIGRATIONS, ...ASSUMPTION_MIGRATIONS, ...DATED_WAIT_MIGRATIONS, ...WAKE_RETRY_MIGRATIONS, ...MERGE_BEHIND_MIGRATIONS, ...RETARGET_PULLS_MIGRATIONS]; +// BB-287: stacked pull requests visible for their whole life. `stacked_pulls` +// is the open pull requests stacked on this binding's own pull request, so the +// binding is told its branch is a base and must not be deleted; +// `stacked_recovered` is the stacked pull requests already woken about after +// closing unmerged with their base branch gone, so the recovery wake is once +// per pull request. Additive and defaulted: an old row reads as neither. +export const STACKED_PULLS_MIGRATIONS = [`ALTER TABLE bindings ADD COLUMN stacked_pulls TEXT NOT NULL DEFAULT '[]'`, `ALTER TABLE bindings ADD COLUMN stacked_recovered TEXT NOT NULL DEFAULT '[]'`]; + +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, ...REVIEWER_MODEL_MIGRATIONS, ...WAKE_SOURCE_MIGRATIONS, ...SERVER_SETTINGS_MIGRATIONS, ...PULL_WATCH_MIGRATIONS, ...STACK_MIGRATIONS, ...ESCALATION_ROUTE_MIGRATIONS, ...DEPS_DECISION_MIGRATIONS, ...WORKER_STOP_REASON_MIGRATIONS, ...ASSUMPTION_MIGRATIONS, ...DATED_WAIT_MIGRATIONS, ...WAKE_RETRY_MIGRATIONS, ...MERGE_BEHIND_MIGRATIONS, ...RETARGET_PULLS_MIGRATIONS, ...STACKED_PULLS_MIGRATIONS]; // ---- values kept as JSON ------------------------------------------------------------ @@ -795,6 +803,10 @@ export type Binding = { mergeBehind: string[]; /** Base pull request keys this binding has already been woken to retarget (BB-264), so a stacked pull request whose base has merged is retargeted once. */ retargetPulls: string[]; + /** Open pull requests stacked on this binding's own pull request (BB-287), so its worker is told not to delete the branch that is their base. */ + stackedPulls: string[]; + /** Stacked pull requests already woken about after closing unmerged with their base branch gone (BB-287), so the recovery wake is once per pull request. */ + stackedRecovered: string[]; /** Entity keys of the bindings whose open pull request this hold waits for (a stack-wait, BB-215): it lifts when every one has opened a pull request, not when they merge. */ holdStack: string[]; /** Entity keys the project steward was last asked to schedule this binding against; asked again only for a new one. */ @@ -904,6 +916,8 @@ function rowToBinding(r: BindingRow): Binding { holdBehind: json(z.array(z.string()), str("hold_behind"), []), mergeBehind: json(z.array(z.string()), str("merge_behind"), []), retargetPulls: json(z.array(z.string()), str("retarget_pulls"), []), + stackedPulls: json(z.array(z.string()), str("stacked_pulls"), []), + stackedRecovered: json(z.array(z.string()), str("stacked_recovered"), []), holdStack: json(z.array(z.string()), str("hold_stack"), []), scheduleAsked: json(z.array(z.string()), str("schedule_asked"), []), clearedToStart: Number(r.cleared_to_start ?? 0) === 1, @@ -974,6 +988,8 @@ const COLUMNS = { holdBehind: "hold_behind", mergeBehind: "merge_behind", retargetPulls: "retarget_pulls", + stackedPulls: "stacked_pulls", + stackedRecovered: "stacked_recovered", holdStack: "hold_stack", scheduleAsked: "schedule_asked", clearedToStart: "cleared_to_start", @@ -1040,6 +1056,7 @@ const VISIBLE_COLUMNS = [ "hold", "hold_behind", "merge_behind", + "stacked_pulls", "hold_stack", "status", "pending", diff --git a/plugins/stewards/wake.ts b/plugins/stewards/wake.ts index 3d4deedb..46ccd535 100644 --- a/plugins/stewards/wake.ts +++ b/plugins/stewards/wake.ts @@ -490,6 +490,16 @@ const mergeHoldLine = (binding: Binding): string => { return `- **MERGE HOLD: behind ${keys.join(", ")}** — the plugin files no Merge item while it stands, so the effort shows as held. It lifts itself, and you are woken, once ${keys.length === 1 ? "that binding has" : "each of them has"} merged its pull requests or ended.`; }; +/** + * The stacked-pull-request line of a wake (BB-287): one or more open pull + * requests are based on this binding's own, so its worker must not delete the + * branch that is their base — the deletion closes them unmerged. + */ +const stackedPullsLine = (binding: Binding): string => { + const keys = binding.stackedPulls; + return `- **BASE OF: ${keys.join(", ")}** — ${keys.length === 1 ? "another open pull request is" : "other open pull requests are"} stacked on this binding's own. Do not delete the branch that is their base: before deleting any branch run \`gh pr list --base --state open\` and leave a branch that is the base of an open pull request.`; +}; + /** * The sentence a no-wake-source paragraph carries when a `failed` watch is * holding the binding (BB-194): a settled watch will not wake the steward on @@ -619,6 +629,7 @@ export function formatWake(input: WakeInput): string { out.push(`- Priority: ${binding.priority ?? "normal"}`); out.push(holdLine(binding, "take no new actions and start no workers until it is lifted.")); if (binding.mergeBehind.length > 0) out.push(mergeHoldLine(binding)); + if (binding.stackedPulls.length > 0) out.push(stackedPullsLine(binding)); out.push(...schedulingLines(binding, input.conflicts ?? [], input.escalations)); out.push(...trialLessonsLines(input.lessons ?? [])); out.push(...assumptionLines(input.assumptions ?? [])); From e97f7c29af96457bebb9c7d10ab5a76af4da6490 Mon Sep 17 00:00:00 2001 From: Dylan Trotter Date: Tue, 10 Nov 2026 06:21:49 -0500 Subject: [PATCH 2/2] stewards, github-watcher: an empty dependency graph is reported as such (BB-289) (#317) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `sourceDependabotAlerts` gains an additive, optional `graphEmpty: string[]`: the repositories whose GraphQL `dependencyGraphManifests(first:1){totalCount}` is zero, so an empty alert list means Dependabot cannot see any manifest rather than that there is nothing open. A failed graph read is not an empty graph: the repo stays unflagged. The contract copy is updated in all three plugins, and the deps-alerts wake and `bb stewards check` now say "dependency graph empty" and name the by-hand toggle (repo Settings → Advanced Security → Dependency graph) instead of "no open alerts". Co-authored-by: Claude Sonnet 5.5 --- plugins/github-watcher/github.ts | 30 +++++++++++++++++++++++ plugins/github-watcher/server.test.ts | 19 +++++++++++++- plugins/github-watcher/server.ts | 18 ++++++++++++-- plugins/github-watcher/source-contract.ts | 8 ++++++ plugins/linear/source-contract.ts | 8 ++++++ plugins/stewards/deps.test.ts | 13 +++++++++- plugins/stewards/deps.ts | 7 ++++++ plugins/stewards/server.test.ts | 30 ++++++++++++++++++++--- plugins/stewards/server.ts | 11 +++++++-- plugins/stewards/source-contract.ts | 8 ++++++ plugins/stewards/wake.ts | 4 +++ 11 files changed, 147 insertions(+), 9 deletions(-) diff --git a/plugins/github-watcher/github.ts b/plugins/github-watcher/github.ts index 50d0e947..49bd9040 100644 --- a/plugins/github-watcher/github.ts +++ b/plugins/github-watcher/github.ts @@ -761,6 +761,36 @@ const severityOf = (value: string | null | undefined): DependabotSeverity => { return (DEPENDABOT_SEVERITIES as readonly string[]).includes(s) ? (s as DependabotSeverity) : "low"; }; +/** + * One repository's dependency-graph manifest count, for the dependency watch's + * "dependency graph empty" flag (BB-289): a graph that has parsed zero + * manifests has an empty Dependabot alert feed for a reason a person must + * turn on by hand, not because there is nothing to find. + */ +export const DEPENDENCY_GRAPH_QUERY = `query($owner: String!, $name: String!) { repository(owner: $owner, name: $name) { nameWithOwner dependencyGraphManifests(first: 1) { totalCount } } }`; + +const dependencyGraphPayload = z.object({ + data: z.object({ repository: z.object({ nameWithOwner: z.string(), dependencyGraphManifests: z.object({ totalCount: z.number() }).nullish() }).nullable() }).nullable(), +}); + +/** + * How many manifests the repository's dependency graph has parsed, or null + * when the repository is not there, not visible, or the field is absent. + * Throws when GitHub refused the read; the caller must not read a failed read + * as an empty graph. + */ +export function parseDependencyGraphCount(payload: unknown): number | null { + const errors = graphqlErrors(payload); + const parsed = dependencyGraphPayload.safeParse(payload); + if (!parsed.success) throw new Error(errors.length > 0 ? `GitHub: ${errors.join("; ")}` : "GitHub answered without data"); + const repository = parsed.data.data?.repository ?? null; + if (repository === null) { + if (errors.length > 0 && errorTypes(payload).some((t) => t !== "NOT_FOUND")) throw new Error(`GitHub: ${errors.join("; ")}`); + return null; + } + return repository.dependencyGraphManifests?.totalCount ?? null; +} + /** * One page of `GET /repos/{owner}/{repo}/dependabot/alerts`, as the shared * contract wants it. `repo` is the name the caller asked with; GitHub's diff --git a/plugins/github-watcher/server.test.ts b/plugins/github-watcher/server.test.ts index d3cc2ea9..e8e0b2e4 100644 --- a/plugins/github-watcher/server.test.ts +++ b/plugins/github-watcher/server.test.ts @@ -38,7 +38,7 @@ const comment = (body: string, createdAt: string, who = "bob") => ({ author: { l // A GitHub whose GraphQL endpoint a test scripts: one repository with one // pull request, whose bb-managed label validates. function fakeGitHub() { - const state = { node: pullNode() as Record | null, fail: false, rejectSubIssues: false, valid: (token: string) => token.startsWith("gho_"), openPulls: [] as Array>, openIssues: [] as Array>, searchIssues: [] as Array>, labels: { "acme/widgets": ["bb-managed"] } as Record, labelCreates: [] as string[], labelCreateStatus: 201, labelCreateMessage: "Validation Failed", dependabotAlerts: [] as unknown[], dependabotPage2: [] as unknown[], dependabotStatus: 200, dependabotMessage: "Forbidden", deployments: [] as unknown[], deploymentStatuses: {} as Record, files: {} as Record, commits: {} as Record, workflowRuns: [] as unknown[], workflowRun: null as unknown, jobs: [] as unknown[], compares: {} as Record }; + const state = { node: pullNode() as Record | null, fail: false, rejectSubIssues: false, valid: (token: string) => token.startsWith("gho_"), openPulls: [] as Array>, openIssues: [] as Array>, searchIssues: [] as Array>, labels: { "acme/widgets": ["bb-managed"] } as Record, labelCreates: [] as string[], labelCreateStatus: 201, labelCreateMessage: "Validation Failed", dependabotAlerts: [] as unknown[], dependabotPage2: [] as unknown[], dependabotStatus: 200, dependabotMessage: "Forbidden", dependencyGraph: 1 as number | null, dependencyGraphFail: false, deployments: [] as unknown[], deploymentStatuses: {} as Record, files: {} as Record, commits: {} as Record, workflowRuns: [] as unknown[], workflowRun: null as unknown, jobs: [] as unknown[], compares: {} as Record }; const calls: Array<{ token: string; variables: Record }> = []; const fetchImpl = vi.fn(async (url: string | URL, init?: RequestInit) => { const token = ((init?.headers as Record).authorization ?? "").replace(/^Bearer /u, ""); @@ -109,6 +109,10 @@ function fakeGitHub() { const v = body.variables as { owner: string; name: string; number: number; label?: string }; if (!body.query.includes("repository(")) return Response.json({ data: { viewer: { login: "dev" } } }); if (`${v.owner}/${v.name}`.toLowerCase() !== "acme/widgets") return Response.json({ data: { viewer: { login: "dev" }, repository: null }, errors: [{ type: "NOT_FOUND", message: "Could not resolve to a Repository" }] }); + if (body.query.includes("dependencyGraphManifests")) { + if (state.dependencyGraphFail) return Response.json({ data: null, errors: [{ type: "FORBIDDEN", message: "Resource not accessible by integration" }] }); + return Response.json({ data: { viewer: { login: "dev" }, repository: { nameWithOwner: "Acme/Widgets", dependencyGraphManifests: state.dependencyGraph === null ? null : { totalCount: state.dependencyGraph } } } }); + } if (body.query.includes("label(name:")) { const has = (state.labels[`${v.owner}/${v.name}`.toLowerCase()] ?? []).some((n) => n.toLowerCase() === String(v.label).toLowerCase()); return Response.json({ data: { viewer: { login: "dev" }, repository: { nameWithOwner: "Acme/Widgets", label: has ? { name: String(v.label) } : null } } }); @@ -678,10 +682,23 @@ describe("stewards' source contract", () => { }, ], errors: [], + graphEmpty: [], }); expect(await w.harness.callRpc("sourceDependabotAlerts", { units: ["acme/gone", "widgets"], state: "open" }, STEWARDS)).toMatchObject({ alerts: [], errors: ["acme/gone: GitHub answered HTTP 404: Not Found", "widgets: not owner/repo"] }); }); + it("names a repository whose dependency graph parsed no manifests, and leaves a failed graph read unflagged (BB-289)", async () => { + const w = await setup(); + w.github.state.dependencyGraph = 0; + expect(await w.harness.callRpc("sourceDependabotAlerts", { units: ["Acme/Widgets"], state: "open" }, STEWARDS)).toMatchObject({ alerts: [], graphEmpty: ["Acme/Widgets"], errors: [] }); + // A non-empty graph is not flagged: an empty alert list then means nothing is open. + w.github.state.dependencyGraph = 37; + expect(await w.harness.callRpc("sourceDependabotAlerts", { units: ["Acme/Widgets"], state: "open" }, STEWARDS)).toMatchObject({ graphEmpty: [] }); + // A graph read the token was refused is not an empty graph: it stays unflagged and adds no error. + w.github.state.dependencyGraphFail = true; + expect(await w.harness.callRpc("sourceDependabotAlerts", { units: ["Acme/Widgets"], state: "open" }, STEWARDS)).toMatchObject({ graphEmpty: [], errors: [] }); + }); + it("names a repository whose alerts the token cannot read (403) without failing the whole call", async () => { const w = await setup(); w.github.state.dependabotStatus = 403; diff --git a/plugins/github-watcher/server.ts b/plugins/github-watcher/server.ts index 3aef82aa..acc8acf2 100644 --- a/plugins/github-watcher/server.ts +++ b/plugins/github-watcher/server.ts @@ -22,6 +22,7 @@ import { z } from "zod"; import { WATCHES_CHANGED } from "./channels.js"; import { createGhTokenReader, type TokenReader } from "./gh.js"; import { + DEPENDENCY_GRAPH_QUERY, FILE_AT_REF_QUERY, GITHUB_API_URL, GitHubRequestError, @@ -43,6 +44,7 @@ import { labelPath, parseCompare, parseDependabotAlerts, + parseDependencyGraphCount, parseDeploymentStatus, parseDeployments, parseFileAtRef, @@ -746,15 +748,21 @@ export function createGitHubPlugin(deps: GitHubPluginDeps): (bb: BbPluginApi) => // alerts disabled, not visible) is named in `errors`; the rest are // still returned. Reading alerts needs more than read access to the // repository (bb-gate/docs/hooks.md, "Admission, alerts"). + // + // One GraphQL read of `dependencyGraphManifests(first: 1) { totalCount }` + // per repository says whether an empty alert list is the truth or a + // dependency graph that has parsed nothing (BB-289); a failed graph read + // is not an empty graph, so it is left unflagged and adds no error. async sourceDependabotAlerts({ units, state }) { let viewer: string; try { viewer = parseViewer(await query(VIEWER_QUERY, {}, checkSignal())); } catch (error) { - return { viewer: null, alerts: [], errors: [errorMessage(error)] }; + return { viewer: null, alerts: [], errors: [errorMessage(error)], graphEmpty: [] }; } const alerts: ReturnType = []; const errors: string[] = []; + const graphEmpty: string[] = []; for (const unit of units) { const repo = parseRepo(unit); if (repo === null) { @@ -774,8 +782,14 @@ export function createGitHubPlugin(deps: GitHubPluginDeps): (bb: BbPluginApi) => } catch (error) { errors.push(`${unit}: ${errorMessage(error)}`); } + try { + if (parseDependencyGraphCount(await query(DEPENDENCY_GRAPH_QUERY, { ...repo }, checkSignal())) === 0) graphEmpty.push(name); + } catch { + // A read the token was refused is not an empty graph: leave the + // repository unflagged rather than claim Dependabot can see nothing. + } } - return { viewer, alerts, errors }; + return { viewer, alerts, errors, graphEmpty }; }, // A repository's recent deployments, newest first, read with the token diff --git a/plugins/github-watcher/source-contract.ts b/plugins/github-watcher/source-contract.ts index 2d152013..5f63bfc2 100644 --- a/plugins/github-watcher/source-contract.ts +++ b/plugins/github-watcher/source-contract.ts @@ -472,6 +472,13 @@ export const sourceContract = { * github plugin reads them with the developer's token from the REST API; * each repository the token cannot read (no alert access, alerts disabled, * not visible) is named in `errors` rather than failing the whole call. + * + * `graphEmpty` names the repositories whose dependency graph parsed no + * manifests (`dependencyGraphManifests.totalCount` is 0), so an empty alert + * list means Dependabot cannot see anything rather than that there is + * nothing to see (BB-289). Additive and optional: a source that does not + * implement it leaves it empty, and a repository whose graph read failed is + * not named (a failed read is not an empty graph). */ sourceDependabotAlerts: { input: z.object({ @@ -483,6 +490,7 @@ export const sourceContract = { viewer: z.string().nullable(), alerts: z.array(dependabotAlertSchema), errors: z.array(z.string()), + graphEmpty: z.array(z.string()).default([]), }), }, /** diff --git a/plugins/linear/source-contract.ts b/plugins/linear/source-contract.ts index 2d152013..5f63bfc2 100644 --- a/plugins/linear/source-contract.ts +++ b/plugins/linear/source-contract.ts @@ -472,6 +472,13 @@ export const sourceContract = { * github plugin reads them with the developer's token from the REST API; * each repository the token cannot read (no alert access, alerts disabled, * not visible) is named in `errors` rather than failing the whole call. + * + * `graphEmpty` names the repositories whose dependency graph parsed no + * manifests (`dependencyGraphManifests.totalCount` is 0), so an empty alert + * list means Dependabot cannot see anything rather than that there is + * nothing to see (BB-289). Additive and optional: a source that does not + * implement it leaves it empty, and a repository whose graph read failed is + * not named (a failed read is not an empty graph). */ sourceDependabotAlerts: { input: z.object({ @@ -483,6 +490,7 @@ export const sourceContract = { viewer: z.string().nullable(), alerts: z.array(dependabotAlertSchema), errors: z.array(z.string()), + graphEmpty: z.array(z.string()).default([]), }), }, /** diff --git a/plugins/stewards/deps.test.ts b/plugins/stewards/deps.test.ts index 1540d7b3..eba59b84 100644 --- a/plugins/stewards/deps.test.ts +++ b/plugins/stewards/deps.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "vitest"; -import { ADOPTION_CHOICES, DEFAULT_BATCH, DEFAULT_SLA, adoptionChoices, adoptionHow, adoptionQuestion, alertClockAt, alertFingerprint, alertLine, batchAlert, dateInText, deadlineAt, depsFindingSchema, dueDate, isBatchable, overdue, priorityFor, reminderText, sightingOf, snoozeUntil } from "./deps.js"; +import { ADOPTION_CHOICES, DEFAULT_BATCH, DEFAULT_SLA, adoptionChoices, adoptionHow, adoptionQuestion, alertClockAt, alertFingerprint, alertLine, batchAlert, dateInText, deadlineAt, depsFindingSchema, depsLookSchema, dueDate, isBatchable, overdue, priorityFor, reminderText, sightingOf, snoozeUntil } from "./deps.js"; import type { DependabotAlert } from "./source-contract.js"; const alert = (over: Partial = {}): DependabotAlert => ({ @@ -198,3 +198,14 @@ describe("the adoption decision", () => { expect(parsed).toMatchObject({ ticketLabels: [], alertCreatedAt: null }); }); }); + +describe("the dependency look", () => { + const look = { at: now, target: "dependabot/acme/web", error: null, viewer: "Jane", alerts: [] }; + + it("keeps the graph-empty flag when present and treats a look stored before it as not empty (BB-289)", () => { + expect(depsLookSchema.parse({ ...look, graphEmpty: true }).graphEmpty).toBe(true); + expect(depsLookSchema.parse({ ...look, graphEmpty: false }).graphEmpty).toBe(false); + // A look written before the field parses; absent means false, so the wake says "no open alerts". + expect(depsLookSchema.parse(look).graphEmpty).toBeUndefined(); + }); +}); diff --git a/plugins/stewards/deps.ts b/plugins/stewards/deps.ts index 8f8173b7..ef351589 100644 --- a/plugins/stewards/deps.ts +++ b/plugins/stewards/deps.ts @@ -202,6 +202,13 @@ export const depsLookSchema = z.object({ error: z.string().nullable(), viewer: z.string().nullable(), alerts: z.array(depsFindingSchema), + /** + * Whether the repository's dependency graph parsed no manifests, so an + * empty alert list means Dependabot cannot see anything rather than that + * there is nothing to see (BB-289). Optional so a look stored before this + * field parses; absent means false. + */ + graphEmpty: z.boolean().optional(), }); export type DepsLook = z.infer; diff --git a/plugins/stewards/server.test.ts b/plugins/stewards/server.test.ts index a4f435c5..94feb65b 100644 --- a/plugins/stewards/server.test.ts +++ b/plugins/stewards/server.test.ts @@ -32,6 +32,8 @@ function world() { const entities = new Map(); /** Dependabot alerts the fake github serves, by lowercased repo. */ const dependabotAlerts = new Map(); + /** Repos whose dependency graph parsed no manifests, lowercased, for the graph-empty wake (BB-289). */ + const dependabotGraphEmpty = new Set(); /** Entity ids whose snapshot read throws, for the failed-read test (BB-115). */ const snapshotErrors = new Set(); /** The fake linear dedup search: unsupported (github-style decline) or an error, for BB-136. */ @@ -318,7 +320,7 @@ function world() { } else if (args.method === "sourceDependabotAlerts") { const units = input.units as string[]; - out = { viewer: "jane", alerts: units.flatMap((u) => dependabotAlerts.get(u.toLowerCase()) ?? []), errors: [] }; + out = { viewer: "jane", alerts: units.flatMap((u) => dependabotAlerts.get(u.toLowerCase()) ?? []), errors: [], graphEmpty: units.filter((u) => dependabotGraphEmpty.has(u.toLowerCase())) }; } else if (args.method === "sourceDeployments") { const key = `${String(input.repo).toLowerCase()} ${String(input.environment).toLowerCase()}`; @@ -543,7 +545,7 @@ function world() { }, }; let now = Date.parse("2026-10-14T15:00:00Z"); - return { files, writes, writtenContent, fileContents, repos, primary, entities, projectList, dependabotAlerts, sourceDeployments, sourceFiles, sourceCommits, sourceRuns, sourceCompares, labelEnsureCalls, labelsEnsured, labelEnsureFailures, setLabelEnsureUnavailable: (value: boolean) => { labelEnsureUnavailable = value; }, setPushUnavailable: (value: boolean) => { pushUnavailable = value; }, setLabelEnsureThrow: (value: string | null) => { labelEnsureThrow = value; }, setLayout: (value: "primary" | "directory") => { layout = value; primary = value === "directory" ? null : { dir: "bb", url: "https://github.com/acme/bb.git" }; }, viewers, setLinearMine: (teams: Array<{ unit: string; name: string }>) => { linearMine = teams; }, setMissingStates: (source: "linear" | "github", unit: string, states: string[]) => { missingStates.set(`${source}:${unit.toLowerCase()}`, states); }, setMissingLabel: (source: "linear" | "github", unit: string, labels: string[]) => { missingLabels.set(`${source}:${unit.toLowerCase()}`, labels); }, rpcCalls, githubTrees, githubBranchProtection, githubLabels, setSnapshotError: (id: string, fail: boolean) => { if (fail) snapshotErrors.add(id); else snapshotErrors.delete(id); }, setSearchUnsupported: (value: boolean) => { searchUnsupported = value; }, setSearchError: (value: string | null) => { searchError = value; }, setCreatePullError: (value: string | null) => { createPullError = value; }, setMergePullError: (value: string | null) => { mergePullError = value; }, setMoveHeadOnMerge: (value: boolean) => { moveHeadOnMerge = value; }, setPullDetailsError: (value: string | null) => { pullDetailsError = value; }, setRetryConflict: (value: boolean) => { retryConflict = value; }, setRetryQueued: (value: boolean) => { retryQueued = value; }, threads, deletes, sections, usage, metadata, spawns, sends, retries, retryEvents, queuedDeletes, userTurns, githubComments, githubReviews, githubMerges, githubPulls, githubClosedPulls, githubDefaultBranches, githubBranches, githubFiles, githubFileContents, githubDispatches, setMergeDispatchError: (value: string | null) => { mergeDispatchError = value; }, github, sdk, uiPreferences, uiPreferencesWrites, now: () => now, advance: (ms: number) => (now += ms) }; + return { files, writes, writtenContent, fileContents, repos, primary, entities, projectList, dependabotAlerts, dependabotGraphEmpty, sourceDeployments, sourceFiles, sourceCommits, sourceRuns, sourceCompares, labelEnsureCalls, labelsEnsured, labelEnsureFailures, setLabelEnsureUnavailable: (value: boolean) => { labelEnsureUnavailable = value; }, setPushUnavailable: (value: boolean) => { pushUnavailable = value; }, setLabelEnsureThrow: (value: string | null) => { labelEnsureThrow = value; }, setLayout: (value: "primary" | "directory") => { layout = value; primary = value === "directory" ? null : { dir: "bb", url: "https://github.com/acme/bb.git" }; }, viewers, setLinearMine: (teams: Array<{ unit: string; name: string }>) => { linearMine = teams; }, setMissingStates: (source: "linear" | "github", unit: string, states: string[]) => { missingStates.set(`${source}:${unit.toLowerCase()}`, states); }, setMissingLabel: (source: "linear" | "github", unit: string, labels: string[]) => { missingLabels.set(`${source}:${unit.toLowerCase()}`, labels); }, rpcCalls, githubTrees, githubBranchProtection, githubLabels, setSnapshotError: (id: string, fail: boolean) => { if (fail) snapshotErrors.add(id); else snapshotErrors.delete(id); }, setSearchUnsupported: (value: boolean) => { searchUnsupported = value; }, setSearchError: (value: string | null) => { searchError = value; }, setCreatePullError: (value: string | null) => { createPullError = value; }, setMergePullError: (value: string | null) => { mergePullError = value; }, setMoveHeadOnMerge: (value: boolean) => { moveHeadOnMerge = value; }, setPullDetailsError: (value: string | null) => { pullDetailsError = value; }, setRetryConflict: (value: boolean) => { retryConflict = value; }, setRetryQueued: (value: boolean) => { retryQueued = value; }, threads, deletes, sections, usage, metadata, spawns, sends, retries, retryEvents, queuedDeletes, userTurns, githubComments, githubReviews, githubMerges, githubPulls, githubClosedPulls, githubDefaultBranches, githubBranches, githubFiles, githubFileContents, githubDispatches, setMergeDispatchError: (value: string | null) => { mergeDispatchError = value; }, github, sdk, uiPreferences, uiPreferencesWrites, now: () => now, advance: (ms: number) => (now += ms) }; } async function setup(extra: Partial = {}, settings: Record = {}, seedProvider?: string | null | "fail-list" | "fail-set", seedNavigation?: string | null | "fail-list" | "fail-set") { @@ -9815,13 +9817,14 @@ describe("dependency stewards", () => { ...over, }); - async function watching(alerts: unknown[] = [alert()], project: Record = {}, entities: Array<[string, EntitySnapshot]> = []) { + async function watching(alerts: unknown[] = [alert()], project: Record = {}, entities: Array<[string, EntitySnapshot]> = [], graphEmpty = false) { const t = await setup(); // The fixture's primary (acme/bb) is a stewarded repo too; these tests watch one repo. t.files[PROJECT_FILE] = JSON.stringify({ version: 1, linear: { teams: ["ENG"] }, github: { exclude: ["acme/bb"] }, ...project }); t.files[playbookPath("deps-alerts")] = DEPS_ALERTS; t.files[playbookPath("deps-prs")] = DEPS_PRS; t.dependabotAlerts.set("acme/web", alerts); + if (graphEmpty) t.dependabotGraphEmpty.add("acme/web"); for (const [id, entity] of entities) t.entities.set(id, entity); await t.hooks.sweep(); await t.flush(); @@ -9949,6 +9952,20 @@ describe("dependency stewards", () => { expect(wakes().at(-1)).toMatch(/deps\.autoAdopt: a ticket you file for a critical or high alert is adopted at once/u); }); + it("says the dependency graph is empty, naming the toggle, instead of 'no open alerts' when the graph parsed nothing (BB-289)", async () => { + const { wakes } = await watching([], {}, [], true); + const first = wakes()[0]!; + expect(first).toMatch(/\*\*Dependency graph empty\.\*\*/u); + expect(first).toMatch(/Dependabot cannot see any manifest/u); + expect(first).toMatch(/Settings → Advanced Security → Dependency graph/u); + expect(first).not.toMatch(/\*\*No open alerts\.\*\*/u); + + // A repo whose graph is readable still says "no open alerts" when there are none. + const { wakes: readable } = await watching([]); + expect(readable()[0]).toMatch(/\*\*No open alerts\.\*\*/u); + expect(readable()[0]).not.toMatch(/Dependency graph empty/u); + }); + it("replays SC-111: the adoption decision is durable plugin state, the SLA clock is the alert's, and reminders land in needs and the digest", async () => { // Two medium alerts with no fix, opened 2026-10-10; medium SLA 7d → due 2026-10-16 (America/New_York). const echo = (number: number, ghsaId: string) => alert({ id: `acme/web#${number}`, number, severity: "medium", package: "github.com/labstack/echo/v4", ecosystem: "go", manifestPath: "go.mod", ghsaId, fixedVersion: null, createdAt: "2026-10-10T00:19:43Z", url: `https://github.com/acme/web/security/dependabot/${number}` }); @@ -10305,6 +10322,13 @@ describe("dependency stewards", () => { expect(check.stdout).toMatch(/deps-prs: deps-prs@/u); expect(check.stdout).toMatch(/deps: watching 1 repo for alerts: acme\/web/u); }); + + it("check says a repo's dependency graph is empty, naming the toggle (BB-289)", async () => { + const { t } = await watching([], {}, [], true); + const check = await t.harness.behavior.runCli(["check"], { projectId: "p1" }); + expect(check.stdout).toMatch(/deps: acme\/web: dependency graph empty/u); + expect(check.stdout).toMatch(/Settings → Advanced Security → Dependency graph/u); + }); }); // BB-235: a project with no Linear team files ops and Dependabot tickets as diff --git a/plugins/stewards/server.ts b/plugins/stewards/server.ts index 83a9cd57..c9b52f56 100644 --- a/plugins/stewards/server.ts +++ b/plugins/stewards/server.ts @@ -4376,6 +4376,9 @@ export function createStewardsPlugin(deps: StewardsDeps): (bb: BbPluginApi) => P try { const out = await sourceRpc("github", "sourceDependabotAlerts", { units: [target.repo], state: "open" }, sourceContract.sourceDependabotAlerts.output); const alerts: DependabotAlert[] = out.alerts; + // The source names a repo whose dependency graph parsed no manifests, + // so an empty alert list is not "nothing open" (BB-289). + const graphEmpty = out.graphEmpty.some((r) => r.toLowerCase() === target.repo.toLowerCase()); const rows = store.sight( b.projectId, b.id, @@ -4441,11 +4444,11 @@ export function createStewardsPlugin(deps: StewardsDeps): (bb: BbPluginApi) => P batch: batchAlert(a, dueAt, open), }); } - const look: DepsLook = { at, target: opsTargetKey(target), error: null, viewer: out.viewer, alerts: depsFindings }; + const look: DepsLook = { at, target: opsTargetKey(target), error: null, viewer: out.viewer, alerts: depsFindings, graphEmpty }; reconcileDepsDecisions(b, config, look, allRows, tickets, byFingerprint, clockAt); return look; } catch (error) { - return { at, target: opsTargetKey(target), error: errorMessage(error), viewer: null, alerts: [] }; + return { at, target: opsTargetKey(target), error: errorMessage(error), viewer: null, alerts: [], graphEmpty: false }; } } @@ -11241,6 +11244,10 @@ export function createStewardsPlugin(deps: StewardsDeps): (bb: BbPluginApi) => P const out = await sourceRpc("github", "sourceDependabotAlerts", { units: depsRepos.map((t) => t.repo), state: "open" }, sourceContract.sourceDependabotAlerts.output); if (out.viewer !== null) lines.push(`deps: reading alerts as ${out.viewer}`); for (const e of out.errors) lines.push(`deps: skipping ${e}: the gh login cannot read its alerts (a skip, not a problem)`); + // A repo whose dependency graph parsed nothing has an empty alert + // feed for a reason a person turns on by hand, not because there is + // nothing open (BB-289). + for (const repo of out.graphEmpty) lines.push(`deps: ${repo}: dependency graph empty — Dependabot cannot see any manifest, so "no open alerts" means nothing; turn it on in the repository's Settings → Advanced Security → Dependency graph`); } catch (error) { lines.push(`problem: deps: could not read alerts: ${errorMessage(error)}`); } diff --git a/plugins/stewards/source-contract.ts b/plugins/stewards/source-contract.ts index 2d152013..5f63bfc2 100644 --- a/plugins/stewards/source-contract.ts +++ b/plugins/stewards/source-contract.ts @@ -472,6 +472,13 @@ export const sourceContract = { * github plugin reads them with the developer's token from the REST API; * each repository the token cannot read (no alert access, alerts disabled, * not visible) is named in `errors` rather than failing the whole call. + * + * `graphEmpty` names the repositories whose dependency graph parsed no + * manifests (`dependencyGraphManifests.totalCount` is 0), so an empty alert + * list means Dependabot cannot see anything rather than that there is + * nothing to see (BB-289). Additive and optional: a source that does not + * implement it leaves it empty, and a repository whose graph read failed is + * not named (a failed read is not an empty graph). */ sourceDependabotAlerts: { input: z.object({ @@ -483,6 +490,7 @@ export const sourceContract = { viewer: z.string().nullable(), alerts: z.array(dependabotAlertSchema), errors: z.array(z.string()), + graphEmpty: z.array(z.string()).default([]), }), }, /** diff --git a/plugins/stewards/wake.ts b/plugins/stewards/wake.ts index 46ccd535..685e2334 100644 --- a/plugins/stewards/wake.ts +++ b/plugins/stewards/wake.ts @@ -1034,6 +1034,10 @@ export function formatDepsWake(input: DepsWakeInput): string { const head = out.splice(0); out.push("", "## 3. The alerts", ""); if (look.error !== null) out.push(`The repository could not be read: ${look.error}. Nothing to do until it can; \`bb stewards check\` says why.`); + else if (look.alerts.length === 0 && look.graphEmpty === true) + out.push( + "**Dependency graph empty.** Dependabot cannot see any manifest for this repository, so an empty alert list is not evidence that there is nothing to fix. Turn the dependency graph on by hand in the repository's Settings → Advanced Security → Dependency graph; the next look will then read real alerts. Nothing to file, comment on or close: report that with steward_report (actor other) and end the turn.", + ); else if (look.alerts.length === 0) out.push("**No open alerts.** Nothing to file, comment on or close: report that with steward_report (actor other) and end the turn."); for (const c of present) { out.push("", `### ${DEPS_CLASS_TITLES[c]}`, "");