stewards: an open PR is not a wake source; gate-met stalls escalate (BB-285)

plugins/github-watcher/server.test.ts

@@ -870,6 +870,46 @@ describe("stewards' source contract", () => {
       await service.done;
     });
 
+    // BB-286: a reconnect re-check rings stewards for every repo whose
+    // deployment doorbells it holds, so a deployment doorbell dropped while
+    // the subscriber was away is re-read rather than waiting for the sweep.
+    it("rings stewards for each deployment interest repo on the connected sweep (BB-286)", async () => {
+      const w = await setup(WITH_HOOKS);
+      const service = w.harness.runService("watches");
+      await tick();
+      w.hooks.last().send(w.hooks.hello("c_1"));
+      await tick();
+      expect(await w.harness.callRpc("sourceSetInterest", { units: [], entities: [], dependabot: [], deployment: ["acme/bb-gate"] }, STEWARDS)).toEqual({ ok: true });
+      await tick();
+      // A new connection re-checks; the deployment interest is rung once.
+      w.hooks.last().send(w.hooks.hello("c_2"));
+      await tick(3000);
+      expect(w.rpcs()).toEqual([{ pluginId: "stewards", method: "stewardsRing", input: { source: "github", id: "acme/bb-gate" } }]);
+      service.controller.abort();
+      await service.done;
+    });
+
+    // BB-286: the deployment queue holds the latest object per deployment id,
+    // so two statuses inside the debounce hand stewards the newest, not the
+    // first.
+    it("hands stewards the newest of two deployment statuses inside the debounce (BB-286)", async () => {
+      const w = await setup(WITH_HOOKS);
+      const service = w.harness.runService("watches");
+      await tick();
+      w.hooks.last().send(w.hooks.hello("c_1"));
+      await tick();
+      expect(await w.harness.callRpc("sourceSetInterest", { units: [], entities: [], dependabot: [], deployment: ["acme/bb-gate"] }, STEWARDS)).toEqual({ ok: true });
+      await tick();
+      const inProgress = { id: 7, environment: "production", ref: "main", sha: "abc123", state: "in_progress", url: "https://github.com/acme/bb-gate/actions/runs/9" };
+      const success = { ...inProgress, state: "success" };
+      w.hooks.last().send(`event: doorbell\ndata: ${JSON.stringify({ source: "github", kind: "deployment", id: "Acme/bb-gate#7", unit: "Acme/bb-gate", deployment: inProgress })}\n\n`);
+      w.hooks.last().send(`event: doorbell\ndata: ${JSON.stringify({ source: "github", kind: "deployment", id: "Acme/bb-gate#7", unit: "Acme/bb-gate", deployment: success })}\n\n`);
+      await tick(3000);
+      expect(w.rpcs()).toEqual([{ pluginId: "stewards", method: "stewardsRing", input: { source: "github", id: "Acme/bb-gate#7", deployment: success } }]);
+      service.controller.abort();
+      await service.done;
+    });
+
     it("leases what stewards asked for after a restart, beside the watches", async () => {
       const w = await setup({ ...WITH_HOOKS, kv: { "stewards/interest": { units: ["acme/gadgets"], entities: ["acme/widgets#12"] } } });
       await w.tool({ ref: "Acme/Widgets#12" });

plugins/github-watcher/server.ts

@@ -323,6 +323,13 @@ export function createGitHubPlugin(deps: GitHubPluginDeps): (bb: BbPluginApi) =>
       const keys = watches.keys();
       if (keys.length > 0) bb.log.info(`checking ${keys.length} watched GitHub pull request${keys.length === 1 ? "" : "s"} or issue${keys.length === 1 ? "" : "s"} (${reason})`);
       await checks.sweep(keys);
+      // BB-286: a reconnect re-check must also ring stewards for every repo
+      // whose deployment doorbells it holds. A deployment doorbell dropped
+      // while this server was away is otherwise only found by the half-hourly
+      // sweep; the ring makes stewards re-read that repo's deployments now.
+      if (reason === "connected") {
+        for (const repo of interest.deployment) rings.ring(repo);
+      }
     }
 
     // ---- what stewards wants to hear about ------------------------------------

plugins/stewards/deploy.test.ts

@@ -255,6 +255,23 @@ describe("recording merged pull requests", () => {
     expect(recorded["bitcomplete/bb-gate#20::bb-gate"]).toMatchObject({ state: "merged", mergeSha: "stacked1", deadlineAt: NOW + liveAtMergeTarget.timeout });
   });
 
+  // BB-286: a watch recorded under an older playbook is re-matched against
+  // the current one on every read, so a rule that is now live-at-merge
+  // settles it with no deployment read. The first-record contract above still
+  // holds when the rule has not changed.
+  it("re-matches a pre-existing merged watch when the playbook's rule becomes live-at-merge (BB-286)", () => {
+    const refTarget: DeploymentTarget = { name: "bb-gate", source: "github", repo: "bitcomplete/bb-gate", environment: "production", carries: [{ when: { repo: "acme/web" }, strategy: { kind: "ref-contains-merge" } }], timeout: 3_600_000, workflows: [] };
+    const liveTarget: DeploymentTarget = { ...refTarget, carries: [{ when: { repo: "acme/web", paths: [".bb/**"] }, strategy: { kind: "live-at-merge" } }] };
+    const mergedPull = { entity: pull({ key: "acme/web#7" }, { merged: true, mergeCommit: "aaaaaaa1", files: [".bb/stewards/project.json"] }), pulls: [] };
+    const before = recordMerged({}, mergedPull, [refTarget], NOW);
+    expect(before["acme/web#7::bb-gate"]).toMatchObject({ state: "merged", deadlineAt: NOW + refTarget.timeout });
+    const after = recordMerged(before, mergedPull, [liveTarget], NOW + 1000);
+    expect(after["acme/web#7::bb-gate"]).toMatchObject({ state: "deployed", deadlineAt: null, note: "live-at-merge: nothing deploys .bb/**" });
+    // No deployment read is needed: nothing times out and nothing awaits.
+    expect(timeoutsDue(after, NOW + liveTarget.timeout)).toEqual([]);
+    expect(awaitingOnRepo(after, "bitcomplete/bb-gate")).toEqual([]);
+  });
+
   it("does not record an open pull request, and reads a ticket's linked merged ones", () => {
     expect(recordMerged({}, observed({ entity: pull({ key: "acme/web#7" }, { merged: false }) }), [target], NOW)).toEqual({});
     const linked = recordMerged({}, observed({ entity: pull({ key: "acme/web#7", kind: "pull" }, { merged: false }), pulls: [pull({ key: "acme/web#8" }, { merged: true })] }), [target], NOW);

plugins/stewards/deploy.ts

@@ -19,6 +19,13 @@ import type { Observed } from "./store.js";
 export const DEPLOY_STATES = ["merged", "deploying", "deployed", "failed"] as const;
 export type DeployState = (typeof DEPLOY_STATES)[number];
 
+/**
+ * Whether a deployment's own last status is final (BB-286). A non-final
+ * status (`in_progress`, `created`, `queued`, the empty state) has not
+ * applied the image yet, so a watch that last read one keeps being polled.
+ */
+export const isFinalDeploymentState = (state: string | null | undefined): boolean => state === "success" || state === "failure" || state === "error";
+
 export const deploymentWatchSchema = z.object({
   /** The pull request's key, e.g. "acme/web#12"; the map's key as well. */
   key: z.string(),
@@ -113,6 +120,13 @@ function liveVerdict(target: DeploymentTarget, prRepo: string, files: readonly s
  * the source now knows one and the stored one differs (a watch recorded
  * before BB-132 holds the pull request head, which a squash merge rewrites).
  *
+ * An existing watch is also re-matched against the playbook's current rule
+ * on every read (BB-286): a watch recorded under an older playbook keeps the
+ * old rule otherwise. When the rule that now matches its stored `prRepo` and
+ * `files` is `live-at-merge` and the merge is on the default branch, the
+ * watch settles `deployed` with the live note and no deadline, so no
+ * deployment is read for it. A stacked merge stays awaiting.
+ *
  * A target whose structured `carries` has no rule for the merge is not
  * applicable and is skipped; a watch already stored for it is dropped, so it
  * cannot block closing the ticket. When no target applies at all, every
@@ -195,6 +209,22 @@ export function recordMerged(
           next[id] = { ...existing, state: reevaluate ? "deploying" : existing.state, mergeSha: merge, note };
           changed = true;
         }
+        // BB-286: re-match the watch against the playbook's current rule.
+        // The stored `prRepo` and `files` decide applicability, as they do
+        // above. When the matching rule is now `live-at-merge` and the merge
+        // is on the default branch, it is live and settles with the live note
+        // and no deadline, without any deployment read. A stacked merge
+        // (`onDefault` false) keeps awaiting its own merge to reach `main`.
+        const current = next[id] ?? existing;
+        if (onDefault && current.state !== "deployed" && current.state !== "failed" && typeof target.carries !== "string") {
+          const context = contextFor(target);
+          const matched = matchCarriesRule(target.carries, context.prRepo, context.files);
+          const live = matched === null ? null : liveAtMergeVerdict(matched);
+          if (live !== null) {
+            next[id] = { ...current, state: "deployed", at: now, deadlineAt: null, note: live.note };
+            changed = true;
+          }
+        }
         continue;
       }
       // BB-260: a `live-at-merge` rule names paths nothing builds or applies,

plugins/stewards/reconcile.test.ts

@@ -1068,6 +1068,14 @@ describe("idle in an active stage (BB-83)", () => {
     expect(idleInActiveStage(binding({ requeueUntil: at }), merged, at + 1)).toBe(true);
   });
 
+  // BB-285: a gate-met own pull request is the plugin's to act on, so it does
+  // not suppress the idle wake the way an open one does.
+  it("does not suppress the idle wake for a gate-met own pull request", () => {
+    const open = { entity: issue({ state: "In Review" }), pulls: [pull({ state: "open" })] };
+    expect(idleInActiveStage(binding(), open)).toBe(false);
+    expect(idleInActiveStage(binding({ gateMetPulls: new Set(["acme/web#7"]) }), open)).toBe(true);
+  });
+
   it("starts a new idle period when an own pull request closes", () => {
     const before = { entity: issue({ state: "In Review" }), pulls: [pull({ state: "open" })] };
     expect(ownPullClosed(before, merged, own)).toBe(true);

plugins/stewards/reconcile.ts

@@ -670,6 +670,12 @@ export function unheardWait(status: Binding["status"], open: readonly Escalation
 export type IdleBinding = Pick<Binding, "stage" | "ending" | "hold" | "deployments" | "requeueUntil"> & {
   workers: readonly Pick<Worker, "status">[];
   ownPulls: ReadonlySet<string>;
+  /**
+   * Lowercased keys of the binding's own pull requests whose gate is met
+   * (BB-285). A gate-met own pull request is not something to wait on: the
+   * plugin acts on it, so it does not suppress the idle wake.
+   */
+  gateMetPulls?: ReadonlySet<string>;
   escalations: readonly Pick<Escalation, "status" | "route">[];
   needs: readonly unknown[];
 };
@@ -697,7 +703,7 @@ export function idleInActiveStage(b: IdleBinding, observed: Observed, now: numbe
   // "nothing is being waited on" before then.
   if (b.requeueUntil !== null && b.requeueUntil > now) return false;
   if (b.workers.some((w) => w.status === "running")) return false;
-  if ([observed.entity, ...observed.pulls].some((e) => pullOpen(e) && ownPull(e.key, e.url, b.ownPulls))) return false;
+  if ([observed.entity, ...observed.pulls].some((e) => pullOpen(e) && ownPull(e.key, e.url, b.ownPulls) && !(b.gateMetPulls?.has(e.key.toLowerCase()) ?? false))) return false;
   if (Object.values(b.deployments).some((w) => w.state === "merged" || w.state === "deploying")) return false;
   if (b.escalations.some((x) => x.status === "open" && x.route === "user")) return false;
   if (b.needs.length > 0) return false;