Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions src/review/outcomes-wire.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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
Expand Down
54 changes: 54 additions & 0 deletions test/unit/miner-decision-pack-rebuild-trigger.test.ts
Original file line number Diff line number Diff line change
@@ -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<typeof import("../../src/services/decision-pack")>()),
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");
});
});