diff --git a/src/queue/processors.ts b/src/queue/processors.ts index 385fd800..ec572c9e 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -1466,7 +1466,8 @@ async function reReviewStoredPullRequest( await persistAdvisory(env, advisory); if ( shouldCollectSlopEvidence(settings) || - settings.manifestPolicyGateMode !== "off" + settings.manifestPolicyGateMode !== "off" || + (await shouldRefreshFilesForPreMergeChecks(env, repoFullName)) ) { await refreshPullRequestDetails(env, repoFullName, prNumber).catch( () => undefined, @@ -2908,7 +2909,8 @@ async function processGitHubWebhook( if ( shouldCollectSlopEvidence(settings) || settings.manifestPolicyGateMode !== "off" || - isAgentConfigured(settings.autonomy) + isAgentConfigured(settings.autonomy) || + (await shouldRefreshFilesForPreMergeChecks(env, repoFullName)) ) { await refreshPullRequestDetails(env, repoFullName, pr.number); } @@ -3204,6 +3206,16 @@ export function shouldCollectSlopEvidence( return settings.slopGateMode !== "off" || mergeReadinessGateEnabled(settings); } +export async function shouldRefreshFilesForPreMergeChecks( + env: Env, + repoFullName: string, +): Promise { + const checks = resolveReviewPreMergeChecks( + await loadRepoFocusManifest(env, repoFullName).catch(() => null), + ); + return checks.some((check) => check.whenPaths.length > 0); +} + export function shouldRunSlopAiAdvisory( settings: Pick, ): boolean { @@ -5659,7 +5671,8 @@ async function maybeProcessPrPanelRetrigger( // (#866/#925): refresh before publishing so the re-published Gate check reflects the latest file set. if ( shouldCollectSlopEvidence(settings) || - settings.manifestPolicyGateMode !== "off" + settings.manifestPolicyGateMode !== "off" || + (await shouldRefreshFilesForPreMergeChecks(env, repoFullName)) ) { await refreshPullRequestDetails(env, repoFullName, pr.number); } diff --git a/test/unit/gate-check-policy.test.ts b/test/unit/gate-check-policy.test.ts index 78a2926a..84aaceb3 100644 --- a/test/unit/gate-check-policy.test.ts +++ b/test/unit/gate-check-policy.test.ts @@ -1,11 +1,12 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import { generateKeyPairSync } from "node:crypto"; import { clearInstallationTokenCacheForTest } from "../../src/github/app"; -import { buildAuthorizedPrActionAdvisory, gateCheckPolicy, resolveLinkedIssueAuthorLogins, shouldCollectLinkedIssueEvidence, shouldCollectSlopEvidence, shouldRunSlopAiAdvisory } from "../../src/queue/processors"; +import { buildAuthorizedPrActionAdvisory, gateCheckPolicy, resolveLinkedIssueAuthorLogins, shouldCollectLinkedIssueEvidence, shouldCollectSlopEvidence, shouldRefreshFilesForPreMergeChecks, shouldRunSlopAiAdvisory } from "../../src/queue/processors"; import { createTestEnv } from "../helpers/d1"; import { upsertIssueFromGitHub, upsertRepositoryFromGitHub } from "../../src/db/repositories"; import { evaluateGateCheck } from "../../src/rules/advisory"; import { parseFocusManifest, resolveEffectiveSettings } from "../../src/signals/focus-manifest"; +import { upsertRepoFocusManifest } from "../../src/signals/focus-manifest-loader"; import type { Advisory, PullRequestRecord, RepositorySettings } from "../../src/types"; function settings(over: Partial = {}): RepositorySettings { @@ -266,6 +267,35 @@ describe("merge-readiness evidence collection (#551)", () => { expect(shouldCollectSlopEvidence(settings({ slopGateMode: "off", mergeReadinessGateMode: "off" }))).toBe(false); }); + it("refreshes files when pre-merge checks are path-gated (#review-pre-merge-checks)", async () => { + const env = createTestEnv(); + + expect(await shouldRefreshFilesForPreMergeChecks(env, "JSONbored/gittensory")).toBe(false); + + await upsertRepoFocusManifest(env, "JSONbored/gittensory", { + review: { pre_merge_checks: [{ name: "Approval", require_label: "approved" }] }, + }); + expect(await shouldRefreshFilesForPreMergeChecks(env, "JSONbored/gittensory")).toBe(false); + + await upsertRepoFocusManifest(env, "JSONbored/gittensory", { + review: { pre_merge_checks: [{ name: "Migration approval", require_label: "approved", when_paths: ["migrations/**"] }] }, + }); + expect(await shouldRefreshFilesForPreMergeChecks(env, "JSONbored/gittensory")).toBe(true); + }); + + it("fails safe to false when the focus-manifest load throws (#review-pre-merge-checks)", async () => { + // A manifest-load failure must NOT trigger a refresh — the `.catch(() => null)` fail-safe resolves to an + // empty check set, so `.some()` is false and the path-gated refresh stays off. + const env = createTestEnv({ + DB: { + prepare() { + throw new Error("D1 unavailable"); + }, + } as unknown as Env["DB"], + }); + expect(await shouldRefreshFilesForPreMergeChecks(env, "JSONbored/gittensory")).toBe(false); + }); + it("runs AI slop advisory only when the slop gate is explicitly enabled", () => { expect(shouldRunSlopAiAdvisory(settings({ slopGateMode: "advisory", slopAiAdvisory: true }))).toBe(true); expect(shouldRunSlopAiAdvisory(settings({ slopGateMode: "block", slopAiAdvisory: true }))).toBe(true); diff --git a/test/unit/queue.test.ts b/test/unit/queue.test.ts index 0447fea8..871310f6 100644 --- a/test/unit/queue.test.ts +++ b/test/unit/queue.test.ts @@ -19,6 +19,7 @@ import { listProductUsageDailyRollups, listProductUsageEvents, listPullRequests, + listPullRequestFiles, listRepoSyncStates, listSignalSnapshots, persistSignalSnapshot, @@ -1512,6 +1513,84 @@ describe("queue processors", () => { expect(rcAudit).toBeFalsy(); }); + it("refreshes pull request files for path-gated pre-merge checks on synchronize (#review-pre-merge-checks)", async () => { + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); + await upsertRepositoryFromGitHub(env, { name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } }, 123); + await upsertInstallation(env, { + installation: { + id: 123, + account: { login: "JSONbored", id: 1, type: "User" }, + repository_selection: "selected", + permissions: { metadata: "read", pull_requests: "write", issues: "write" }, + events: ["pull_request"], + }, + repositories: [{ name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } }], + }); + await upsertRepositorySettings(env, { + repoFullName: "JSONbored/gittensory", + commentMode: "off", + publicSurface: "off", + autoLabelEnabled: false, + checkRunMode: "off", + gateCheckMode: "off", + autonomy: { merge: "observe", request_changes: "observe" }, + slopGateMode: "off", + mergeReadinessGateMode: "off", + manifestPolicyGateMode: "off", + }); + await upsertRepoFocusManifest(env, "JSONbored/gittensory", { + review: { pre_merge_checks: [{ name: "Migration approval", require_label: "approved", when_paths: ["migrations/**"], enforce: true }] }, + }); + await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { + number: 49, + title: "feat: add migration", + state: "open", + user: { login: "contributor" }, + head: { sha: "gate125" }, + labels: [], + body: "Closes #1", + }); + await upsertPullRequestFile(env, { repoFullName: "JSONbored/gittensory", pullNumber: 49, path: "src/feature.ts", status: "modified", additions: 1, deletions: 0, changes: 1, payload: {} }); + + let pullFilesFetches = 0; + vi.stubGlobal("fetch", async (input: RequestInfo | URL) => { + const url = input.toString(); + if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + if (url.includes("/pulls/49/files")) { + pullFilesFetches += 1; + return Response.json([{ filename: "migrations/0099_security.sql", status: "added", additions: 3, deletions: 0, changes: 3 }]); + } + if (url.includes("/pulls/49/reviews")) return Response.json([]); + if (url.includes("/commits/gate125/check-runs")) return Response.json({ total_count: 0, check_runs: [] }); + if (url.includes("/commits/gate125/status")) return Response.json({ statuses: [] }); + return new Response("not found", { status: 404 }); + }); + + await processJob(env, { + type: "github-webhook", + deliveryId: "pre-merge-refresh-sync", + eventName: "pull_request", + payload: { + action: "synchronize", + installation: { id: 123, account: { login: "JSONbored", id: 1, type: "User" } }, + repository: { name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } }, + pull_request: { + number: 49, + title: "feat: add migration", + state: "open", + user: { login: "contributor" }, + head: { sha: "gate125" }, + labels: [], + body: "Closes #1", + mergeable_state: "clean", + }, + }, + }); + + expect(pullFilesFetches).toBeGreaterThan(0); + expect((await listPullRequestFiles(env, "JSONbored/gittensory", 49)).map((file) => file.path)).toEqual(["migrations/0099_security.sql"]); + }); + it("pre-merge checks (#review-pre-merge-checks): an enforced check that fails blocks the auto-merge", async () => { const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); await persistRegistrySnapshot( @@ -2062,6 +2141,33 @@ describe("queue processors", () => { ).resolves.toBeUndefined(); }); + it("recapture-preview (#review-pre-merge-checks): a slop-gated re-review refreshes the PR's files before publishing", async () => { + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); + await upsertInstallation(env, { action: "created", installation: { id: 9102, account: { login: "owner", id: 1, type: "Organization" }, target_type: "Organization", repository_selection: "selected", permissions: {}, events: [] } }); + await upsertRepositoryFromGitHub(env, { name: "slop-repo", full_name: "owner/slop-repo", private: false, owner: { login: "owner" } }, 9102); + // slopGateMode != "off" ⇒ shouldCollectSlopEvidence(settings) is true ⇒ reReviewStoredPullRequest enters the + // refresh branch (the file-refresh body), so the stored files reflect the PR's current head before publishing. + await upsertRepositorySettings(env, { repoFullName: "owner/slop-repo", checkRunMode: "off", commentMode: "off", publicSurface: "off", slopGateMode: "advisory" }); + await upsertPullRequestFromGitHub(env, "owner/slop-repo", { number: 11, title: "Slop PR", state: "open", user: { login: "contributor" }, head: { sha: "s11" }, labels: [], body: "x" }); + vi.stubGlobal("fetch", async (input: RequestInfo | URL) => { + const url = input.toString(); + if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + if (url.includes("/pulls/11/files")) return Response.json([{ filename: "src/a.ts", status: "modified", additions: 1, deletions: 0, changes: 1, patch: "@@\n+export const ok = true;" }]); + if (/\/pulls\/11(?:\?|$)/.test(url)) return Response.json({ number: 11, title: "Slop PR", state: "open", user: { login: "contributor" }, head: { sha: "s11" }, labels: [], body: "x" }); + if (url.includes("/commits/s11/check-runs")) return Response.json({ total_count: 0, check_runs: [] }); + if (url.includes("/commits/s11/status")) return Response.json({ state: "success", statuses: [] }); + return new Response("not found", { status: 404 }); + }); + + await expect( + processJob(env, { type: "recapture-preview", deliveryId: "rp-11", installationId: 9102, repoFullName: "owner/slop-repo", prNumber: 11, attempt: 1 }), + ).resolves.toBeUndefined(); + + // refreshPullRequestDetails ran ⇒ a detail-sync-state row was written for this PR (the if-body executed). + const sync = await env.DB.prepare("select status from pull_request_detail_sync_state where repo_full_name = ? and pull_number = ?").bind("owner/slop-repo", 11).first<{ status: string }>(); + expect(sync?.status).toMatch(/^(complete|partial)$/); + }); + it("auto-maintain (#778): a repo with no acting autonomy takes no agent action", async () => { const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); await persistRegistrySnapshot(