stewards, github-watcher: a deployment is re-read after a non-final status; a merged watch is re-matched (BB-286)

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/server.test.ts

@@ -12121,6 +12121,105 @@ describe("deployment watching (BB-53)", () => {
     expect(t.hooks.store.find("ENG-12")!.pending.some((n) => n.quiet === true && /created/.test(n.text))).toBe(true);
   });
 
+  // BB-286: a non-final doorbell used to note quietly and stop; a dropped
+  // final doorbell then waited for the half-hourly sweep. The doorbell path
+  // now schedules a re-read, so the final status is caught within the
+  // debounce.
+  it("re-reads after a non-final deployment doorbell and settles on the final status (BB-286)", async () => {
+    const t = await setup();
+    merged(t);
+    await t.ring("linear", "uuid-12");
+    // The doorbell is in_progress, but the deployment source already has the
+    // success: the scheduled re-read settles the watch with no second doorbell.
+    t.sourceDeployments.set("acme/bb-gate production", [deployment()]);
+    await doorbell(t, "acme/bb-gate#7", deployment({ state: "in_progress" }));
+    await t.flush();
+    expect(t.hooks.store.find("ENG-12")!.deployments["acme/web#7"]).toMatchObject({ state: "deploying", deploymentState: "success", deploymentId: 7 });
+  });
+
+  // BB-286: the re-read repeats while the last read was non-final. The first
+  // re-read still sees in_progress; the poll settles it when it turns success
+  // later, rather than waiting for the half-hourly sweep.
+  it("keeps polling while the deployment is non-final and settles when it turns final (BB-286)", async () => {
+    const t = await setup({ timing: { debounceMs: 1, nightlyStaggerMs: 0, deploymentPollMs: 20 } });
+    merged(t);
+    await t.ring("linear", "uuid-12");
+    // The source is still in_progress for the doorbell and the first re-read.
+    t.sourceDeployments.set("acme/bb-gate production", [deployment({ state: "in_progress" })]);
+    await doorbell(t, "acme/bb-gate#7", deployment({ state: "in_progress" }));
+    await t.flush();
+    expect(t.hooks.store.find("ENG-12")!.deployments["acme/web#7"]).toMatchObject({ state: "merged", deploymentState: "in_progress" });
+    // The final status lands later; the poll re-reads and settles it.
+    t.sourceDeployments.set("acme/bb-gate production", [deployment({ state: "success" })]);
+    await new Promise((r) => setTimeout(r, 30));
+    await t.flush();
+    expect(t.hooks.store.find("ENG-12")!.deployments["acme/web#7"]).toMatchObject({ state: "deploying", deploymentState: "success", deploymentId: 7 });
+  });
+
+  // BB-286: the same re-read on the sweep read path.
+  it("re-reads after a non-final deployment status found on the sweep (BB-286)", async () => {
+    const t = await setup();
+    merged(t);
+    await t.ring("linear", "uuid-12");
+    t.sourceDeployments.set("acme/bb-gate production", [deployment({ state: "in_progress" })]);
+    await t.hooks.sweep();
+    // The sweep noted in_progress and scheduled a re-read; the source has the
+    // final status by the time it runs.
+    t.sourceDeployments.set("acme/bb-gate production", [deployment()]);
+    await t.flush();
+    expect(t.hooks.store.find("ENG-12")!.deployments["acme/web#7"]).toMatchObject({ state: "deploying", deploymentState: "success", deploymentId: 7 });
+  });
+
+  // BB-286: the deploy debounce keys on the deployment id and state, so two
+  // statuses inside the window both run and the newest wins. A first-wins
+  // debounce kept only the in_progress closure and dropped the success.
+  it("keeps the newest of two deployment statuses inside the debounce (BB-286)", async () => {
+    const t = await setup();
+    merged(t);
+    await t.ring("linear", "uuid-12");
+    const send = (state: string) =>
+      t.harness.callRpc("stewardsRing", { source: "github", id: "acme/bb-gate#7", deployment: deployment({ state }) }, { experimental_caller: { kind: "plugin", pluginId: "github-watcher" } });
+    // Both inside the same debounce window, before either timer fires.
+    await Promise.all([send("in_progress"), send("success")]);
+    await t.flush();
+    expect(t.hooks.store.find("ENG-12")!.deployments["acme/web#7"]).toMatchObject({ state: "deploying", deploymentState: "success", deploymentId: 7 });
+  });
+
+  // BB-286: a watch recorded under an older rule is re-matched against the
+  // playbook on every read. When the rule is now live-at-merge it settles with
+  // the live note and no deadline, and the steward hears "Deployed at merge".
+  it("settles a pre-existing merged watch when the playbook's rule becomes live-at-merge (BB-286)", async () => {
+    const t = await setup();
+    t.files[playbookPath("linear-tickets")] = structuredPlaybook("    - when: { repo: acme/web }\n      strategy: { kind: ref-contains-merge }");
+    t.entities.set("uuid-12", issue({ id: "uuid-12", key: "ENG-12", state: "In Progress", linkedPulls: [{ url: "https://github.com/acme/web/pull/7", key: "acme/web#7" }] }));
+    t.entities.set("acme/web#7", pull({ key: "acme/web#7" }, { merged: true, mergeCommit: "aaaaaaa1", files: [".bb/stewards/project.json"] }));
+    await t.ring("linear", "uuid-12");
+    expect(t.hooks.store.find("ENG-12")!.deployments["acme/web#7"]).toMatchObject({ state: "merged" });
+    const steward = t.spawns[1]!.id;
+    // The rule becomes live-at-merge; the next read settles it without any
+    // deployment read. Forget the cached config so the new playbook is read.
+    t.files[playbookPath("linear-tickets")] = structuredPlaybook('    - when: { repo: acme/web, paths: [.bb/**] }\n      strategy: { kind: live-at-merge }');
+    t.hooks.forgetConfigs();
+    await t.ring("linear", "uuid-12");
+    expect(t.hooks.store.find("ENG-12")!.deployments["acme/web#7"]).toMatchObject({ state: "deployed", deadlineAt: null, note: "live-at-merge: nothing deploys .bb/**" });
+    const wake = t.sends.filter((s) => s.threadId === steward).at(-1)!.text;
+    expect(wake).toMatch(/Deployed at merge: acme\/web#7/u);
+  });
+
+  // BB-286: github-watcher's reconnect sweep rings stewards once per repo with
+  // a deployment interest, and the repo ring re-reads that repo's deployments
+  // (BB-59) rather than waiting for the half-hourly sweep.
+  it("re-reads deployments for a repo when github-watcher rings it on reconnect (BB-286)", async () => {
+    const t = await setup();
+    merged(t);
+    await t.ring("linear", "uuid-12");
+    expect(t.hooks.store.find("ENG-12")!.deployments["acme/web#7"]).toMatchObject({ state: "merged" });
+    // The final deployment already exists at the source when the ring arrives.
+    t.sourceDeployments.set("acme/bb-gate production", [deployment()]);
+    await t.ring("github", "acme/bb-gate");
+    expect(t.hooks.store.find("ENG-12")!.deployments["acme/web#7"]).toMatchObject({ state: "deploying", deploymentId: 7, deploymentState: "success" });
+  });
+
   // BB-204: a success whose image is behind the merge leaves the watch where
   // it was and does not wake the steward.
   it("keeps a success whose image is behind the merge quiet (BB-204)", async () => {