diff --git a/src/review/outcomes-wire.ts b/src/review/outcomes-wire.ts index 2f7fcc76ba..ba22c683b2 100644 --- a/src/review/outcomes-wire.ts +++ b/src/review/outcomes-wire.ts @@ -24,6 +24,7 @@ // once a repo's merge precision actually drops below the floor over a real sample. import { recordAuditEvent } from "../db/repositories"; +import { tryEnqueueDecisionPackRebuild } from "../services/decision-pack"; import { incr } from "../selfhost/metrics"; import type { GitHubWebhookPayload } from "../types"; import { errorMessage, nowIso } from "../utils/json"; @@ -337,6 +338,22 @@ export async function recordPrOutcome( ), ); + // #4283: proactively refresh THIS PR author's decision pack now (within seconds) instead of waiting up to + // DECISION_PACK_MAX_AGE_MS (~6h) for the next passive staleness read at serving time. Best-effort + non-blocking — + // an enqueue failure must never affect pr_outcome recording (mirrors the caller's own `.catch` at processors.ts). + // A non-authoritative self-close already returned above, so this only fires on real outcomes; skip an empty login. + // The 6h passive check stays as the fallback ceiling for authors this proactive path misses. + if (authorLogin) { + await tryEnqueueDecisionPackRebuild(env, authorLogin).catch((error) => + console.warn( + JSON.stringify({ + event: "pr_outcome_decision_pack_rebuild_error", + message: errorMessage(error).slice(0, 160), + }), + ), + ); + } + // Discord/Slack action notifications are emitted by the action executor, which knows the exact bot action that // was attempted and can audit the delivery. This outcome recorder only stores realized ground truth. Emitting // another webhook from the GitHub `pull_request.closed` event duplicated bot-action notifications and could diff --git a/test/unit/miner-decision-pack-rebuild-trigger.test.ts b/test/unit/miner-decision-pack-rebuild-trigger.test.ts new file mode 100644 index 0000000000..6cc8313895 --- /dev/null +++ b/test/unit/miner-decision-pack-rebuild-trigger.test.ts @@ -0,0 +1,54 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { createTestEnv } from "../helpers/d1"; +import type { GitHubWebhookPayload } from "../../src/types"; + +// Spy on the one function this issue wires up; keep every other decision-pack export real so outcomes-wire's +// transitive graph is unaffected. +const { enqueueSpy } = vi.hoisted(() => ({ enqueueSpy: vi.fn() })); +vi.mock("../../src/services/decision-pack", async (importActual) => ({ + ...(await importActual()), + tryEnqueueDecisionPackRebuild: enqueueSpy, +})); + +import { recordPrOutcome } from "../../src/review/outcomes-wire"; + +const closedPr = (opts: { author: string; sender: string; merged: boolean; senderType?: string }) => + ({ + action: "closed", + pull_request: { number: 7, merged_at: opts.merged ? "2026-01-01T00:00:00Z" : null, user: { login: opts.author } }, + repository: { full_name: "acme/widgets" }, + sender: { login: opts.sender, type: opts.senderType ?? "User" }, + }) as unknown as GitHubWebhookPayload; + +describe("recordPrOutcome → proactive decision-pack rebuild (#4283)", () => { + beforeEach(() => enqueueSpy.mockReset().mockResolvedValue(true)); + + it("enqueues a rebuild for the PR author on a merged (maintainer-authoritative) close", async () => { + const env = createTestEnv(); + await recordPrOutcome(env, "pull_request", closedPr({ author: "Miner1", sender: "maintainer", merged: true })); + expect(enqueueSpy).toHaveBeenCalledWith(env, "miner1"); // authorLogin, lowercased + }); + + it("does NOT enqueue on a self-close (author closes their own unmerged PR — anti-poisoning suppression)", async () => { + const env = createTestEnv(); + await recordPrOutcome(env, "pull_request", closedPr({ author: "miner1", sender: "miner1", merged: false })); + expect(enqueueSpy).not.toHaveBeenCalled(); + }); + + it("does NOT enqueue when the PR has no author login (ghost/deleted account)", async () => { + const env = createTestEnv(); + // merged (so the self-close guard doesn't apply), but no user login ⇒ authorLogin is empty ⇒ skip the enqueue + const payload = { action: "closed", pull_request: { number: 7, merged_at: "2026-01-01T00:00:00Z", user: null }, repository: { full_name: "acme/widgets" }, sender: { login: "maintainer", type: "User" } } as unknown as GitHubWebhookPayload; + await recordPrOutcome(env, "pull_request", payload); + expect(enqueueSpy).not.toHaveBeenCalled(); + }); + + it("a rebuild-enqueue failure never throws out of recordPrOutcome", async () => { + const env = createTestEnv(); + enqueueSpy.mockRejectedValueOnce(new Error("boom")); + await expect( + recordPrOutcome(env, "pull_request", closedPr({ author: "miner1", sender: "bot", senderType: "Bot", merged: true })), + ).resolves.toBeUndefined(); + expect(enqueueSpy).toHaveBeenCalledWith(env, "miner1"); + }); +});