Do not start a second review while one is running

apps/api/src/api/activity/activity.service.ts

@@ -422,6 +422,28 @@ export class ActivityService {
         return rows[0]?.headSha ?? null;
     }
 
+    /**
+     * True while a pull request review of this repository and pull request is
+     * still running. The completed check alone lets a second event started
+     * before the first review ends launch a duplicate review: GitHub sends
+     * "opened" and a following "synchronize" seconds apart, and both would pass.
+     * A run left in "started" by a crash is reset to "failed" at boot.
+     */
+    public async hasRunningPullRequestReview(repositoryId: number, targetIid: number): Promise<boolean> {
+        const [{ total }] = await db
+            .select({ total: count() })
+            .from(activityTable)
+            .where(
+                and(
+                    eq(activityTable.repositoryId, repositoryId),
+                    eq(activityTable.targetIid, targetIid),
+                    eq(activityTable.type, "pr_review"),
+                    eq(activityTable.status, "started"),
+                ),
+            );
+        return total > 0;
+    }
+
     public async hasCompletedPullRequestReview(repositoryId: number, targetIid: number): Promise<boolean> {
         const [{ total }] = await db
             .select({ total: count() })

apps/api/src/api/activity/pr-review-guard.test.ts

@@ -0,0 +1,113 @@
+import { beforeEach, describe, expect, it } from "bun:test";
+import { resolve } from "node:path";
+
+// Isolate the real database from module mocks in other test files.
+if (process.env.PROVAL_PR_REVIEW_GUARD_TEST_CHILD !== "1") {
+    it("passes pull request review guard integration in an isolated process", () => {
+        const result = Bun.spawnSync([process.execPath, "test", import.meta.path], {
+            env: { ...process.env, PROVAL_PR_REVIEW_GUARD_TEST_CHILD: "1", DB_FILE_NAME: ":memory:" },
+            stdout: "pipe",
+            stderr: "pipe",
+        });
+        if (result.exitCode !== 0) {
+            throw new Error(result.stdout.toString() + result.stderr.toString());
+        }
+        expect(result.exitCode).toBe(0);
+    }, 30000);
+} else {
+    process.env.DB_FILE_NAME = ":memory:";
+    const { migrate } = await import("drizzle-orm/bun-sqlite/migrator");
+    const { activityTable, repositoryTable } = await import("@proval/db");
+    const { default: db } = await import("../../db/index.js");
+    // Loaded before the service: log.ts builds an ActivityService at module load.
+    await import("../../util/log.js");
+    const { ActivityService } = await import("./activity.service.js");
+
+    migrate(db, { migrationsFolder: resolve(import.meta.dir, "../../../../../packages/db/src/migration") });
+    const service = new ActivityService();
+
+    type ActivityType = "pr_review" | "pr_reply" | "issue_open" | "issue_reply";
+    type ActivityStatus = "started" | "completed" | "failed" | "canceled";
+
+    let repositoryId: number;
+
+    async function addActivity(
+        options: {
+            type?: ActivityType;
+            status?: ActivityStatus;
+            targetIid?: number;
+            repositoryId?: number;
+        } = {},
+    ) {
+        await db.insert(activityTable).values({
+            repositoryId: options.repositoryId ?? repositoryId,
+            repositoryPath: "group/repo",
+            provider: "github",
+            modelName: "test-model",
+            type: options.type ?? "pr_review",
+            status: options.status ?? "started",
+            targetIid: options.targetIid ?? 8,
+            logVersion: "1",
+        });
+    }
+
+    beforeEach(async () => {
+        await db.delete(activityTable);
+        await db.delete(repositoryTable);
+        const [repository] = await db
+            .insert(repositoryTable)
+            .values({ path: "group/repo", provider: "github" })
+            .returning({ id: repositoryTable.id });
+        repositoryId = repository.id;
+    });
+
+    describe("hasRunningPullRequestReview", () => {
+        it("reports a review that is still in flight", async () => {
+            await addActivity({ status: "started" });
+
+            expect(await service.hasRunningPullRequestReview(repositoryId, 8)).toBe(true);
+        });
+
+        it("ignores a review that already ended, whatever its outcome", async () => {
+            for (const status of ["completed", "failed", "canceled"] as const) {
+                await db.delete(activityTable);
+                await addActivity({ status });
+
+                expect(await service.hasRunningPullRequestReview(repositoryId, 8)).toBe(false);
+            }
+        });
+
+        it("ignores another activity type, another pull request and another repository", async () => {
+            await addActivity({ type: "pr_reply", status: "started" });
+            await addActivity({ type: "issue_reply", status: "started" });
+            await addActivity({ targetIid: 9, status: "started" });
+
+            const [otherRepository] = await db
+                .insert(repositoryTable)
+                .values({ path: "group/other", provider: "github" })
+                .returning({ id: repositoryTable.id });
+            await addActivity({ repositoryId: otherRepository.id, status: "started" });
+
+            expect(await service.hasRunningPullRequestReview(repositoryId, 8)).toBe(false);
+        });
+    });
+
+    describe("hasCompletedPullRequestReview", () => {
+        it("stays false while the only review of that pull request is still running", async () => {
+            await addActivity({ status: "started" });
+
+            // This is the gap the running guard closes: two events seconds apart
+            // (opened, then a push) both see no completed review.
+            expect(await service.hasCompletedPullRequestReview(repositoryId, 8)).toBe(false);
+            expect(await service.hasRunningPullRequestReview(repositoryId, 8)).toBe(true);
+        });
+
+        it("becomes true once a review completed", async () => {
+            await addActivity({ status: "started" });
+            await addActivity({ status: "completed" });
+
+            expect(await service.hasCompletedPullRequestReview(repositoryId, 8)).toBe(true);
+            expect(await service.hasRunningPullRequestReview(repositoryId, 8)).toBe(true);
+        });
+    });
+}

apps/api/src/webhook/forgejo/forgejo.controller.ts

@@ -177,6 +177,10 @@ const handleForgejoPullRequestWebhook = async (
     const activityService = new ActivityService();
     const prNumber = pr.number;
     const reviewMode = repository.prReviewOnPush;
+    if (await activityService.hasRunningPullRequestReview(repository.id, prNumber)) {
+        log(`Skipped: review already running (${prInfo})`, "Forgejo");
+        return new Response(JSON.stringify({ message: "Skipped: review already running" }), { status: 200 });
+    }
 
     let hasCompleted: boolean | null = null;
     if (reviewMode === "on_first_push") {

apps/api/src/webhook/github/github.controller.ts

@@ -158,6 +158,10 @@ async function handlePullRequestWebhook(
     if (accessSkip) return accessSkip;
 
     const activityService = new ActivityService();
+    if (await activityService.hasRunningPullRequestReview(repository.id, prNumber)) {
+        return new Response(JSON.stringify({ message: "Skipped: review already running" }), { status: 200 });
+    }
+
     let hasCompleted: boolean | null = null;
     if (reviewMode === "on_first_push") {
         hasCompleted = await activityService.hasCompletedPullRequestReview(repository.id, prNumber);

apps/api/src/webhook/gitlab/gitlab.controller.ts

@@ -144,6 +144,9 @@ const handleGitLabPullRequestWebhook: HandleGitLabPullRequestWebhook = async (
 
     const activityService = new ActivityService();
     const prIid = pullRequest.iid;
+    if (await activityService.hasRunningPullRequestReview(repository.id, prIid)) {
+        return new Response(JSON.stringify({ message: "Skipped: review already running" }), { status: 200 });
+    }
 
     let hasCompleted: boolean | null = null;
     if (reviewMode === "on_first_push") {