From 0143a2b485a84c62a78b1f46218ce0f78684ee9a Mon Sep 17 00:00:00 2001 From: nghetienhiep <13849419+nghetienhiep@users.noreply.github.com> Date: Thu, 16 Jul 2026 17:08:06 +0000 Subject: [PATCH] fix(notifications): enrich disposition notifications with the gate verdict MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The action executor's per-repo Discord/Slack notification used the plain disposition reason (action.reason) as its summary. resolveDispositionReason — built to surface the AI's recorded gate verdict for exactly this notification — was left with no call site once #2881 migrated notifications off the pull_request.closed path and onto the executor, so the enriched, verdict-aware reason its doc comment promises never reached users. Wire resolveDispositionReason into the executor's notify path so a terminal merge/close/request_changes notification shows the latest recorded gate_decision summary for the PR, falling back to the plain disposition reason when no verdict is on record or the read fails. Closes #6636 --- src/services/agent-action-executor.ts | 8 ++++++- test/unit/agent-action-executor.test.ts | 31 +++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/src/services/agent-action-executor.ts b/src/services/agent-action-executor.ts index f2c8c21f83..0a544d612e 100644 --- a/src/services/agent-action-executor.ts +++ b/src/services/agent-action-executor.ts @@ -38,6 +38,7 @@ import { } from "../settings/moderation-rules"; import { incr } from "../selfhost/metrics"; import { shouldWaitForOlderSiblings } from "../review/merge-train"; +import { resolveDispositionReason } from "../review/outcomes-wire"; import { captureError } from "../selfhost/sentry"; // The agent actor name on every audit record — the App acts on the maintainer's behalf per their configured @@ -555,7 +556,12 @@ export async function executeAgentMaintenanceActions(env: Env, ctx: AgentActionE const notifyOutcome: NotifyOutcome | null = action.actionClass === "merge" ? "merged" : action.actionClass === "close" ? "closed" : action.actionClass === "request_changes" ? "manual" : null; if (notifyOutcome) { - const notifyParams = { repoFullName: ctx.repoFullName, pullNumber: ctx.pullNumber, outcome: notifyOutcome, summary: action.reason, submitter: ctx.authorLogin }; + // #6636: surface the AI's actual gate reasoning — the reasonCode summary on the most recent gate_decision + // row for this PR — as the notification reason, falling back to the plain disposition reason when no + // verdict is on record or the read fails. This is the live consumer resolveDispositionReason was built for + // (its own doc comment promises the "enriched, verdict-aware reason the user actually sees"). + const dispositionReason = await resolveDispositionReason(env, targetKey, action.reason); + const notifyParams = { repoFullName: ctx.repoFullName, pullNumber: ctx.pullNumber, outcome: notifyOutcome, summary: dispositionReason, submitter: ctx.authorLogin }; await notifyActionToDiscord(env, notifyParams).catch(() => undefined); await notifyActionToSlack(env, notifyParams).catch(() => undefined); } diff --git a/test/unit/agent-action-executor.test.ts b/test/unit/agent-action-executor.test.ts index 7be7add609..5d6ee1f141 100644 --- a/test/unit/agent-action-executor.test.ts +++ b/test/unit/agent-action-executor.test.ts @@ -48,6 +48,11 @@ vi.mock("../../src/github/backfill", async (importOriginal) => ({ fetchLivePullRequestState: vi.fn(async () => "open" as const), refreshInstallationHealthForInstallation: vi.fn(async () => null), })); +vi.mock("../../src/services/notify-discord", async (importOriginal) => ({ + ...(await importOriginal()), + notifyActionToDiscord: vi.fn(async () => undefined), + notifyActionToSlack: vi.fn(async () => undefined), +})); import { closeIssue, closePullRequest, createIssueComment, createPullRequestReview, dismissLatestBotApproval, mergePullRequest, updatePullRequestBranch } from "../../src/github/pr-actions"; import { ensurePullRequestLabel, removePullRequestLabel } from "../../src/github/labels"; @@ -55,6 +60,7 @@ import { ensurePullRequestAssignee } from "../../src/github/assignees"; import { fetchPullRequestFreshness } from "../../src/github/pr-freshness"; import { createInstallationToken } from "../../src/github/app"; import { fetchLiveCiAggregate, fetchLivePullRequestMergeState, fetchLivePullRequestState, fetchLiveReviewThreadBlockers, refreshInstallationHealthForInstallation } from "../../src/github/backfill"; +import { notifyActionToDiscord, notifyActionToSlack } from "../../src/services/notify-discord"; import { actionParams, applyModerationEscalationForRule, @@ -417,6 +423,31 @@ describe("executeAgentMaintenanceActions (#778 gate stack)", () => { expect(mergePullRequest).toHaveBeenCalled(); }); + it("#6636: a terminal disposition notification uses the recorded gate verdict as its reason when one is on record", async () => { + const env = createTestEnv({}); + // The latest gate_decision row for this PR carries the AI's reasoning — the enriched reason resolveDispositionReason surfaces. + await env.DB.prepare( + "INSERT INTO review_audit (id, project, target_id, event_type, decision, source, head_sha, summary, created_at) VALUES (?,?,?,?,?,?,?,?,?)", + ) + .bind("gate:owner/repo#7", "owner/repo", "owner/repo#7", "gate_decision", "close", "gittensory-native", "sha7", "An AI reviewer flagged a likely blocking defect", "2026-06-21T00:00:00.000Z") + .run(); + + const outcomes = await executeAgentMaintenanceActions(env, ctx(), [merge]); + expect(outcomes[0]?.outcome).toBe("completed"); + expect(mergePullRequest).toHaveBeenCalled(); + // The rendered notification shows the gate verdict, NOT the plain disposition reason ("clean"). + expect(notifyActionToDiscord).toHaveBeenCalledWith(env, expect.objectContaining({ outcome: "merged", summary: "An AI reviewer flagged a likely blocking defect" })); + expect(notifyActionToSlack).toHaveBeenCalledWith(env, expect.objectContaining({ outcome: "merged", summary: "An AI reviewer flagged a likely blocking defect" })); + }); + + it("#6636: the disposition notification falls back to the plain disposition reason when no gate verdict is on record", async () => { + const env = createTestEnv({}); + const outcomes = await executeAgentMaintenanceActions(env, ctx(), [merge]); + expect(outcomes[0]?.outcome).toBe("completed"); + expect(notifyActionToDiscord).toHaveBeenCalledWith(env, expect.objectContaining({ outcome: "merged", summary: merge.reason })); + expect(notifyActionToSlack).toHaveBeenCalledWith(env, expect.objectContaining({ outcome: "merged", summary: merge.reason })); + }); + it("honors a CUSTOM configured manualReviewLabel name (case-insensitive) instead of only the literal default", async () => { const env = createTestEnv({}); vi.mocked(fetchPullRequestFreshness).mockResolvedValueOnce({ status: "current", liveHeadSha: "sha7", liveState: "open", liveLabels: ["Needs-Human"] });