fix(orb): record PR outcomes from the bot's own action, not the webhook alone

scripts/backfill-pr-outcomes-core.ts

@@ -0,0 +1,82 @@
+// Backfill missing pr_outcome rows (#8823) — PURE planning core.
+//
+// The webhook-only pr_outcome writer lost ground truth whenever a `pull_request.closed` delivery was never
+// processed. Because the fleet export inner-joins gate_decision to pr_outcome, every affected PR fell out of
+// calibration entirely — and since the losses skew toward the gate's MISTAKES (a superseded close is by
+// definition a wrong close), their absence biased published accuracy upward.
+//
+// recordTerminalActionOutcome (src/review/outcomes-wire.ts) stops the bleeding going forward; this closes the
+// historical hole. The completed terminal action in audit_events IS the ground truth: `agent.action.merge`
+// means the bot merged it, `agent.action.close` means the bot closed it. Both are authoritative — GitHub
+// rejects a merge the actor may not perform, and a completed close is the repository's realized decision.
+//
+// PURE: decides WHAT to write from rows the caller supplies. The thin IO wrapper reads/writes the ledger.
+
+/** A completed terminal action, as read from audit_events. */
+export type TerminalActionRow = {
+  targetKey: string; // "owner/repo#123"
+  eventType: string; // agent.action.merge | agent.action.close
+  createdAt: string;
+};
+
+export type BackfillPlanEntry = {
+  targetKey: string;
+  project: string;
+  pullNumber: number;
+  decision: "merged" | "closed";
+  createdAt: string;
+};
+
+export type BackfillPlan = {
+  entries: BackfillPlanEntry[];
+  skipped: { alreadyRecorded: number; unparseable: number; unknownAction: number };
+};
+
+/** Split "owner/repo#123" into its parts; null when the shape isn't exactly that. */
+export function parseTargetKey(targetKey: string): { project: string; pullNumber: number } | null {
+  const hash = targetKey.lastIndexOf("#");
+  if (hash <= 0 || hash === targetKey.length - 1) return null;
+  const project = targetKey.slice(0, hash);
+  // Reject the legacy `project:pull_request:owner/repo#123` shape outright: those rows predate the
+  // convergence cutover and the fleet export deliberately excludes them (source != 'gittensory-native').
+  if (!/^[^/:\s]+\/[^/:\s]+$/.test(project)) return null;
+  const pullNumber = Number(targetKey.slice(hash + 1));
+  if (!Number.isInteger(pullNumber) || pullNumber <= 0) return null;
+  return { project, pullNumber };
+}
+
+/**
+ * Plan the backfill: one pr_outcome per terminal-action target that has none.
+ *
+ * When a target somehow carries BOTH a merge and a close action (a close that later got superseded and
+ * re-merged under the same number is not possible on GitHub, but a retry storm could record both), the
+ * LATEST action wins — it is the one whose effect survived.
+ */
+export function planPrOutcomeBackfill(actions: TerminalActionRow[], alreadyRecorded: ReadonlySet<string>): BackfillPlan {
+  const skipped = { alreadyRecorded: 0, unparseable: 0, unknownAction: 0 };
+  const latest = new Map<string, BackfillPlanEntry>();
+
+  for (const action of actions) {
+    const decision = action.eventType === "agent.action.merge" ? "merged" : action.eventType === "agent.action.close" ? "closed" : null;
+    if (decision === null) {
+      skipped.unknownAction += 1;
+      continue;
+    }
+    if (alreadyRecorded.has(action.targetKey)) {
+      skipped.alreadyRecorded += 1;
+      continue;
+    }
+    const parsed = parseTargetKey(action.targetKey);
+    if (parsed === null) {
+      skipped.unparseable += 1;
+      continue;
+    }
+    const existing = latest.get(action.targetKey);
+    if (existing !== undefined && existing.createdAt >= action.createdAt) continue;
+    latest.set(action.targetKey, { targetKey: action.targetKey, project: parsed.project, pullNumber: parsed.pullNumber, decision, createdAt: action.createdAt });
+  }
+
+  // Deterministic order so a dry run and the real run report identically.
+  const entries = [...latest.values()].sort((a, b) => a.createdAt.localeCompare(b.createdAt) || a.targetKey.localeCompare(b.targetKey));
+  return { entries, skipped };
+}

scripts/backfill-pr-outcomes.ts

@@ -0,0 +1,61 @@
+#!/usr/bin/env node
+// Backfill missing pr_outcome rows (#8823) — thin IO wrapper around backfill-pr-outcomes-core.ts.
+//
+// Runs INSIDE a self-hosted instance (it talks to the local ledger, not the cloud): reads completed terminal
+// actions from audit_events, subtracts targets that already have a pr_outcome, and writes the missing rows.
+// Idempotent — re-running writes nothing new.
+//
+//   node --experimental-strip-types scripts/backfill-pr-outcomes.ts --dry-run
+//   node --experimental-strip-types scripts/backfill-pr-outcomes.ts --apply
+//
+// After applying, rewind orb_export_cursor so the corrected rows re-export to fleet calibration:
+//   UPDATE orb_export_cursor SET last_exported_at = '<before the oldest backfilled action>';
+import { planPrOutcomeBackfill, type TerminalActionRow } from "./backfill-pr-outcomes-core.js";
+
+type Row = Record<string, unknown>;
+type Db = {
+  all: (sql: string, binds?: unknown[]) => Promise<Row[]>;
+  run: (sql: string, binds?: unknown[]) => Promise<unknown>;
+};
+
+const TERMINAL_ACTIONS_SQL = `
+  SELECT target_key AS "targetKey", event_type AS "eventType", created_at AS "createdAt"
+    FROM audit_events
+   WHERE event_type IN ('agent.action.merge', 'agent.action.close')
+     AND outcome IN ('success', 'completed')
+   ORDER BY created_at ASC`;
+
+const RECORDED_SQL = `SELECT DISTINCT target_id AS "targetId" FROM review_audit WHERE event_type = 'pr_outcome'`;
+
+export async function runBackfill(db: Db, apply: boolean, log: (line: string) => void = console.log): Promise<number> {
+  const actionRows = (await db.all(TERMINAL_ACTIONS_SQL)) as unknown as TerminalActionRow[];
+  const recordedRows = await db.all(RECORDED_SQL);
+  const recorded = new Set(recordedRows.map((r) => String(r.targetId)));
+
+  const plan = planPrOutcomeBackfill(actionRows, recorded);
+  log(
+    `backfill-pr-outcomes: ${actionRows.length} terminal action(s), ${recorded.size} already recorded -> ` +
+      `${plan.entries.length} to write (skipped: ${plan.skipped.alreadyRecorded} recorded, ` +
+      `${plan.skipped.unparseable} unparseable, ${plan.skipped.unknownAction} non-terminal)`,
+  );
+  if (!apply) {
+    for (const entry of plan.entries.slice(0, 20)) log(`  would write ${entry.targetKey} -> ${entry.decision}`);
+    if (plan.entries.length > 20) log(`  ... and ${plan.entries.length - 20} more`);
+    log("backfill-pr-outcomes: DRY RUN — pass --apply to write");
+    return plan.entries.length;
+  }
+
+  let written = 0;
+  for (const entry of plan.entries) {
+    // review_audit is the store the fleet export and computeGateEval read; the audit_events mirror carries the
+    // same provenance the live writer stamps, tagged so a backfilled row is distinguishable from a live one.
+    await db.run(
+      `INSERT INTO review_audit (id, project, target_id, event_type, decision, source, created_at)
+       VALUES (?, ?, ?, 'pr_outcome', ?, 'gittensory-native', ?)`,
+      [`backfill-${entry.targetKey}-${entry.createdAt}`.slice(0, 190), entry.project.slice(0, 200), entry.targetKey, entry.decision, entry.createdAt],
+    );
+    written += 1;
+  }
+  log(`backfill-pr-outcomes: wrote ${written} pr_outcome row(s)`);
+  return written;
+}

src/review/outcomes-wire.ts

@@ -346,6 +346,61 @@ export async function resolveDispositionReason(
   }
 }
 
+/**
+ * Record the realized outcome of a terminal action THIS bot just completed, without waiting for GitHub to
+ * deliver the matching `pull_request.closed` webhook (#8823).
+ *
+ * recordPrOutcome below is webhook-only, so a delivery this instance never processes — a dropped brokered
+ * relay event, a restart mid-delivery — loses that PR's ground truth PERMANENTLY: the fleet export inner-joins
+ * gate_decision to pr_outcome, so an outcome-less PR vanishes from calibration entirely, contributing to
+ * neither numerator nor denominator. Measured on the live self-host, ~55% of bot closes after 2026-07-18 had
+ * no pr_outcome row while the PRs were verifiably closed on GitHub, and because the dropped rows skew toward
+ * the gate's MISTAKES (a superseded close is by definition a wrong close), their absence biased published
+ * accuracy upward.
+ *
+ * Idempotent: skips when a pr_outcome row already exists for the target, so the webhook path (whichever wins
+ * the race) never produces a duplicate. Best-effort — a bookkeeping failure must never fail the action that
+ * already succeeded against GitHub.
+ */
+export async function recordTerminalActionOutcome(
+  env: Env,
+  repoFullName: string,
+  pullNumber: number,
+  decision: "merged" | "closed",
+): Promise<void> {
+  const targetId = reviewAuditTargetId(repoFullName, pullNumber);
+  try {
+    const existing = await env.DB.prepare(
+      "SELECT 1 AS x FROM review_audit WHERE target_id = ? AND event_type = 'pr_outcome' LIMIT 1",
+    )
+      .bind(targetId)
+      .first<{ x: number }>();
+    if (existing) return;
+  } catch (error) {
+    // An unreadable ledger must not suppress the write — a duplicate row is strictly better than a lost
+    // outcome (the fleet export and computeGateEval both read the LATEST pr_outcome per target).
+    console.warn(JSON.stringify({ event: "pr_outcome_direct_probe_error", message: errorMessage(error).slice(0, 160) }));
+  }
+
+  incr("loopover_pr_outcomes_total", { outcome: decision });
+  await appendReviewAudit(env, {
+    project: repoFullName.slice(0, 200),
+    targetId,
+    eventType: "pr_outcome",
+    decision,
+  });
+  await recordAuditEvent(env, {
+    eventType: "pr_outcome",
+    actor: null,
+    targetKey: targetId,
+    outcome: "completed",
+    detail: decision,
+    metadata: { repoFullName, pullNumber, merged: decision === "merged", botWasActor: true, source: "terminal_action" },
+  }).catch((error) =>
+    console.warn(JSON.stringify({ event: "pr_outcome_direct_audit_error", message: errorMessage(error).slice(0, 160) })),
+  );
+}
+
 export async function recordPrOutcome(
   env: Env,
   eventName: string,

src/services/agent-action-executor.ts

@@ -17,7 +17,7 @@ import {
 import { isAuthorBlacklisted } from "../settings/contributor-blacklist";
 import { classifyMergeFailure, isMergeConflictMessage, isNoNewBaseCommitsMessage, MERGE_RETRY_CAP } from "./merge-failure";
 import { notifyActionToDiscord, notifyActionToSlack, type NotifyOutcome } from "./notify-discord";
-import { resolveDispositionReason } from "../review/outcomes-wire";
+import { recordTerminalActionOutcome, resolveDispositionReason } from "../review/outcomes-wire";
 import { cancelInFlightWorkflowRunsForHeadSha, createInstallationToken, githubErrorStatus, isGitHubRateLimitedError } from "../github/app";
 import { fetchLiveCiAggregate, fetchLivePullRequestMergeState, fetchLivePullRequestState, fetchLiveReviewThreadBlockers, refreshInstallationHealthForInstallation } from "../github/backfill";
 import { githubRateLimitAdmissionKeyForToken } from "../github/client";
@@ -1049,6 +1049,10 @@ async function performAction(env: Env, ctx: AgentActionExecutionContext, action:
       // expectedHeadSha == ctx.headSha, so its behavior is unchanged; the fallback covers any unpinned plan.
       const mergeSha = action.expectedHeadSha ?? ctx.headSha;
       await mergePullRequest(env, ctx.installationId, ctx.repoFullName, ctx.pullNumber, { mergeMethod: action.mergeMethod ?? "squash", ...(mergeSha ? { sha: mergeSha } : {}) });
+      // #8823: record ground truth from the action we just completed rather than depending on the inbound
+      // `pull_request.closed` webhook — a delivery this instance never processes used to lose the outcome
+      // permanently, dropping the PR out of fleet calibration entirely. Idempotent against the webhook path.
+      await recordTerminalActionOutcome(env, ctx.repoFullName, ctx.pullNumber, "merged");
       return;
     }
     case "close":
@@ -1057,6 +1061,9 @@ async function performAction(env: Env, ctx: AgentActionExecutionContext, action:
       // instead of stacking a duplicate "why we closed you" every failed cycle.
       if (action.closeComment) await createOrUpdateCloseExplanationComment(env, ctx.installationId, ctx.repoFullName, ctx.pullNumber, action.closeComment, action.closeKind);
       await closePullRequest(env, ctx.installationId, ctx.repoFullName, ctx.pullNumber);
+      // #8823: see the merge case — the bot's own close is authoritative ground truth and must not depend on
+      // a webhook round-trip the instance might never see.
+      await recordTerminalActionOutcome(env, ctx.repoFullName, ctx.pullNumber, "closed");
       return;
     case "update_branch": {
       // update_branch does NOT need the accept-flow-level "unpinned → deny" gate that #2377/#2422 added for

test/unit/backfill-pr-outcomes-core.test.ts

@@ -0,0 +1,141 @@
+import { describe, expect, it } from "vitest";
+import { parseTargetKey, planPrOutcomeBackfill, type TerminalActionRow } from "../../scripts/backfill-pr-outcomes-core";
+import { runBackfill } from "../../scripts/backfill-pr-outcomes";
+
+// #8823: the historical half of the lost-ground-truth fix. The planning core decides WHAT to backfill from
+// completed terminal actions; a wrong decision here would inject fabricated ground truth into calibration,
+// so every skip arm is asserted explicitly.
+describe("parseTargetKey", () => {
+  it("accepts the canonical owner/repo#N shape", () => {
+    expect(parseTargetKey("JSONbored/loopover#8428")).toEqual({ project: "JSONbored/loopover", pullNumber: 8428 });
+  });
+
+  it("rejects the LEGACY project:pull_request:owner/repo#N shape the fleet export already excludes", () => {
+    expect(parseTargetKey("JSONbored/awesome-claude:pull_request:JSONbored/awesome-claude#2516")).toBeNull();
+  });
+
+  it("rejects malformed keys rather than guessing", () => {
+    expect(parseTargetKey("no-hash")).toBeNull();
+    expect(parseTargetKey("#123")).toBeNull(); // no project
+    expect(parseTargetKey("owner/repo#")).toBeNull(); // no number
+    expect(parseTargetKey("owner/repo#abc")).toBeNull(); // non-numeric
+    expect(parseTargetKey("owner/repo#0")).toBeNull(); // PR numbers are 1-based
+    expect(parseTargetKey("ownerrepo#12")).toBeNull(); // missing the slash
+  });
+});
+
+describe("planPrOutcomeBackfill (#8823)", () => {
+  const row = (targetKey: string, eventType: string, createdAt: string): TerminalActionRow => ({ targetKey, eventType, createdAt });
+
+  it("maps merge → merged and close → closed for targets with no recorded outcome", () => {
+    const plan = planPrOutcomeBackfill(
+      [row("o/r#1", "agent.action.merge", "2026-07-20T00:00:00Z"), row("o/r#2", "agent.action.close", "2026-07-21T00:00:00Z")],
+      new Set(),
+    );
+    expect(plan.entries).toEqual([
+      { targetKey: "o/r#1", project: "o/r", pullNumber: 1, decision: "merged", createdAt: "2026-07-20T00:00:00Z" },
+      { targetKey: "o/r#2", project: "o/r", pullNumber: 2, decision: "closed", createdAt: "2026-07-21T00:00:00Z" },
+    ]);
+  });
+
+  it("NEVER re-writes a target that already has a pr_outcome (the webhook got through)", () => {
+    const plan = planPrOutcomeBackfill([row("o/r#1", "agent.action.close", "2026-07-20T00:00:00Z")], new Set(["o/r#1"]));
+    expect(plan.entries).toEqual([]);
+    expect(plan.skipped.alreadyRecorded).toBe(1);
+  });
+
+  it("skips legacy/unparseable target keys and non-terminal action types instead of fabricating an outcome", () => {
+    const plan = planPrOutcomeBackfill(
+      [
+        row("o/r:pull_request:o/r#5", "agent.action.close", "2026-07-20T00:00:00Z"),
+        row("o/r#6", "agent.action.label", "2026-07-20T00:00:00Z"),
+      ],
+      new Set(),
+    );
+    expect(plan.entries).toEqual([]);
+    expect(plan.skipped).toEqual({ alreadyRecorded: 0, unparseable: 1, unknownAction: 1 });
+  });
+
+  it("when a target has BOTH actions the LATEST wins — the effect that survived — regardless of input order", () => {
+    const late = row("o/r#9", "agent.action.merge", "2026-07-22T00:00:00Z");
+    const early = row("o/r#9", "agent.action.close", "2026-07-20T00:00:00Z");
+    expect(planPrOutcomeBackfill([early, late], new Set()).entries[0]).toMatchObject({ decision: "merged" });
+    expect(planPrOutcomeBackfill([late, early], new Set()).entries[0]).toMatchObject({ decision: "merged" });
+    // Equal timestamps keep the first-seen row rather than flapping on input order.
+    const tie = planPrOutcomeBackfill([row("o/r#9", "agent.action.close", "2026-07-22T00:00:00Z"), late], new Set());
+    expect(tie.entries[0]).toMatchObject({ decision: "closed" });
+  });
+
+  it("emits a deterministic (createdAt, then targetKey) order so a dry run matches the real run", () => {
+    const plan = planPrOutcomeBackfill(
+      [
+        row("o/r#3", "agent.action.close", "2026-07-22T00:00:00Z"),
+        row("o/r#1", "agent.action.close", "2026-07-20T00:00:00Z"),
+        row("o/r#2", "agent.action.close", "2026-07-22T00:00:00Z"),
+      ],
+      new Set(),
+    );
+    expect(plan.entries.map((e) => e.targetKey)).toEqual(["o/r#1", "o/r#2", "o/r#3"]);
+  });
+
+  it("returns an empty plan (never throws) for no input at all", () => {
+    expect(planPrOutcomeBackfill([], new Set())).toEqual({ entries: [], skipped: { alreadyRecorded: 0, unparseable: 0, unknownAction: 0 } });
+  });
+});
+
+describe("runBackfill IO wrapper (#8823)", () => {
+  function fakeDb(actions: Array<Record<string, unknown>>, recorded: string[]) {
+    const writes: unknown[][] = [];
+    return {
+      writes,
+      db: {
+        all: async (sql: string) => (sql.includes("audit_events") ? actions : recorded.map((targetId) => ({ targetId }))),
+        run: async (_sql: string, binds?: unknown[]) => {
+          writes.push(binds ?? []);
+          return undefined;
+        },
+      },
+    };
+  }
+
+  it("DRY RUN reports the plan and writes nothing", async () => {
+    const { db, writes } = fakeDb(
+      [{ targetKey: "o/r#1", eventType: "agent.action.close", createdAt: "2026-07-20T00:00:00Z" }],
+      [],
+    );
+    const lines: string[] = [];
+    expect(await runBackfill(db, false, (l) => lines.push(l))).toBe(1);
+    expect(writes).toHaveLength(0);
+    expect(lines.join("\n")).toContain("DRY RUN");
+    expect(lines.join("\n")).toContain("would write o/r#1 -> closed");
+  });
+
+  it("--apply writes exactly the planned rows and is idempotent on a second run", async () => {
+    const actions = [
+      { targetKey: "o/r#1", eventType: "agent.action.close", createdAt: "2026-07-20T00:00:00Z" },
+      { targetKey: "o/r#2", eventType: "agent.action.merge", createdAt: "2026-07-21T00:00:00Z" },
+    ];
+    const first = fakeDb(actions, []);
+    expect(await runBackfill(first.db, true, () => undefined)).toBe(2);
+    expect(first.writes.map((w) => [w[2], w[3]])).toEqual([
+      ["o/r#1", "closed"],
+      ["o/r#2", "merged"],
+    ]);
+    // Re-run with those targets now recorded -> nothing to do.
+    const second = fakeDb(actions, ["o/r#1", "o/r#2"]);
+    expect(await runBackfill(second.db, true, () => undefined)).toBe(0);
+    expect(second.writes).toHaveLength(0);
+  });
+
+  it("truncates the dry-run listing past 20 entries instead of flooding the operator", async () => {
+    const actions = Array.from({ length: 25 }, (_, i) => ({
+      targetKey: `o/r#${i + 1}`,
+      eventType: "agent.action.close",
+      createdAt: `2026-07-${String((i % 9) + 1).padStart(2, "0")}T00:00:00Z`,
+    }));
+    const { db } = fakeDb(actions, []);
+    const lines: string[] = [];
+    expect(await runBackfill(db, false, (l) => lines.push(l))).toBe(25);
+    expect(lines.join("\n")).toContain("and 5 more");
+  });
+});