Run precheck on addition of closing keyword

manifest.json

@@ -1,7 +1,7 @@
 {
   "name": "daemon-pull-review",
   "description": "A highly context aware pull reviewer",
-  "ubiquity:listeners": ["pull_request.opened", "pull_request.ready_for_review"],
+  "ubiquity:listeners": ["pull_request.opened", "pull_request.ready_for_review", "pull_request.edited"],
   "skipBotEvents": true,
   "configuration": {
     "default": {},

src/handlers/closing-keyword.ts

@@ -0,0 +1,65 @@
+import { getLinkedIssues } from "../helpers/gql-queries";
+import { Context } from "../types";
+import { CallbackResult } from "../types/proxy";
+import { PullReviewer } from "./pull-reviewer";
+
+/**
+ * Handler for the `pull_request.edited` webhook event.
+ * It checks whether the pull request body was edited and now includes a closing keyword.
+ * If so, it runs the pull request precheck.
+ *
+ * @param context
+ */
+export async function handlePullRequestEditedEvent(context: Context<"pull_request.edited">): Promise<CallbackResult> {
+  const { payload, logger } = context;
+
+  const newBody = payload.pull_request.body;
+  if (!newBody) {
+    return { status: 200, reason: "Pull request body is empty, Aborting" };
+  }
+  if (!payload.changes.body?.from) {
+    return { status: 200, reason: "Pull request body wasnt edited, Skipping" };
+  }
+
+  const oldBody: string = payload.changes.body.from;
+
+  // Find matches in both the old and new bodies
+  const oldLinkedIssues = extractIssueUrls(oldBody, context.payload.repository.full_name);
+  const newLinkedIssues = new Set((await getLinkedIssues(context)).map((issue) => issue.url));
+
+  if ((newLinkedIssues.size !== 0 && newLinkedIssues.size !== oldLinkedIssues.size) || [...newLinkedIssues].some((url) => !oldLinkedIssues.has(url))) {
+    logger.info("Pull request body edit detected", {
+      oldLinkedIssues: oldLinkedIssues,
+      newLinkedIssues: newLinkedIssues,
+    });
+    const pullReviewer = new PullReviewer(context);
+    return await pullReviewer.performPullPrecheck();
+  }
+  return { status: 200, reason: "No new closing keyword with an issue reference detected in the PR body edit" };
+}
+
+export function extractIssueUrls(pullBody: string, defaultRepo: string): Set<string> {
+  const bodyWithoutComments = pullBody.replace(/<!--[\s\S]*?-->/g, "");
+
+  const pattern =
+    /(?:close|closes|closed|fix|fixes|fixed|resolve|resolves|resolved)\s+(?:(https:\/\/github\.com\/([^/]+\/[^/]+)\/issues\/(\d+))|([^/\s]+\/[^#\s]+)#(\d+)|#(\d+))/gi;
+  const matches = bodyWithoutComments.matchAll(pattern);
+  const issueUrls = new Set<string>();
+
+  for (const match of matches) {
+    const fullUrl = match[1];
+    const repoPath = match[4];
+    const issueNum1 = match[5];
+    const issueNum2 = match[6];
+
+    if (fullUrl) {
+      issueUrls.add(fullUrl);
+    } else if (repoPath) {
+      issueUrls.add(`https://github.com/${repoPath}/issues/${issueNum1}`);
+    } else {
+      issueUrls.add(`https://github.com/${defaultRepo}/issues/${issueNum2}`);
+    }
+  }
+
+  return issueUrls;
+}

src/handlers/pull-reviewer.ts

@@ -13,7 +13,7 @@ export class PullReviewer {
   readonly context: Context;
   private _oneDay = 24 * 60 * 60 * 1000;
 
-  constructor(context: Context<"pull_request.opened" | "pull_request.ready_for_review">) {
+  constructor(context: Context) {
     this.context = context;
   }
 
@@ -32,7 +32,7 @@ export class PullReviewer {
       return { status: 200, reason: logger.info("PR is closed, no action required").logMessage.raw };
     } else if (!(await this.canPerformReview())) {
       return { status: 200, reason: logger.info("Cannot perform review at this time").logMessage.raw };
-    } else if (pull_request.user.id !== this.context.payload.sender.id) {
+    } else if (this.context.payload.sender && pull_request.user.id !== this.context.payload.sender.id) {
       return { status: 200, reason: logger.info("Review wasn't requested by pull author").logMessage.raw };
     } else if (pull_request.author_association === "COLLABORATOR") {
       return { status: 200, reason: logger.info("Review was requested by core team, Skipping").logMessage.raw };
@@ -47,16 +47,18 @@ export class PullReviewer {
    */
   private async _handleCodeReview(): Promise<CallbackResult> {
     const pullReviewData = await this.reviewPull();
-    const { reviewComment, confidenceThreshold } = this.validateReviewOutput(pullReviewData.answer);
+    if (!pullReviewData) {
+      return { status: 200, reason: "Pull review data not found, Skipping automated review" };
+    }
 
+    const { reviewComment, confidenceThreshold } = this.validateReviewOutput(pullReviewData.answer);
     if (confidenceThreshold > 0.5) {
       await this.addThumbsUpReaction();
     } else {
       await this.convertPullToDraft();
       await this.removeThumbsUpReaction();
       await this.submitCodeReview(reviewComment, "REQUEST_CHANGES");
     }
-
     return { status: 200, reason: "Success" };
   }
 
@@ -115,7 +117,7 @@ export class PullReviewer {
     const { number, organization, repository, action, sender } = payload;
     const { owner, name } = repository;
 
-    logger.info(`${organization}/${repository}#${number} - ${action} - ${sender.login} - ${review}`);
+    logger.info(`${organization}/${repository}#${number} - ${action} - ${sender?.login} - ${review}`);
 
     try {
       const response = await this.context.octokit.rest.pulls.createReview({
@@ -210,8 +212,9 @@ export class PullReviewer {
     } = this.context;
 
     const taskNumber = await this.getTaskNumberFromPullRequest(this.context);
-    const issue = await fetchIssue(this.context, taskNumber);
+    if (!taskNumber) return null;
 
+    const issue = await fetchIssue(this.context, taskNumber);
     if (!issue) {
       throw this.context.logger.error(`Error fetching issue, Aborting`, {
         owner: this.context.payload.repository.owner.login,
@@ -300,7 +303,7 @@ export class PullReviewer {
       };
     }
   }
-  async getTaskNumberFromPullRequest(context: Context<"pull_request.opened" | "pull_request.ready_for_review">) {
+  async getTaskNumberFromPullRequest(context: Context) {
     const {
       payload: { pull_request },
     } = context;
@@ -313,8 +316,8 @@ export class PullReviewer {
     });
 
     if (closingIssues.length === 0) {
-      await this.convertPullToDraft();
-      throw this.context.logger.error("You need to link an issue before converting the pull request to ready for review.");
+      this.context.logger.info("You need to link an issue before converting the pull request to ready for review.");
+      return null;
     } else if (closingIssues.length > 1) {
       throw this.context.logger.error("Multiple tasks are linked to this pull request. This needs to be investigated to determine the best way to handle it.", {
         closingIssues,

src/helpers/callback-proxy.ts

@@ -1,3 +1,4 @@
+import { handlePullRequestEditedEvent } from "../handlers/closing-keyword";
 import { PullReviewer } from "../handlers/pull-reviewer";
 import { Context, SupportedEvents } from "../types";
 import { CallbackResult, ProxyCallbacks } from "../types/proxy";
@@ -12,6 +13,7 @@ import { CallbackResult, ProxyCallbacks } from "../types/proxy";
 const callbacks = {
   "pull_request.opened": [(context: Context) => new PullReviewer(context).performPullPrecheck()],
   "pull_request.ready_for_review": [(context: Context) => new PullReviewer(context).performPullPrecheck()],
+  "pull_request.edited": [(context: Context<"pull_request.edited">) => handlePullRequestEditedEvent(context)],
 } as ProxyCallbacks;
 
 export async function callCallbacks<T extends SupportedEvents>(context: Context<T>, eventName: T): Promise<CallbackResult> {

src/helpers/format-spec-and-pull.ts

@@ -10,7 +10,7 @@ export async function createPullSpecContextBlockSection({
   tokenLimits,
   issue,
 }: {
-  context: Context<"pull_request.ready_for_review" | "pull_request.opened">;
+  context: Context;
   tokenLimits: TokenLimits;
   issue: Issue;
 }): Promise<string> {

src/helpers/gql-queries.ts

@@ -1,4 +1,5 @@
 import { PullRequest } from "@octokit/graphql-schema";
+import { Context } from "../types/context";
 
 type ClosedByPullRequestsReferences = {
   node: Pick<PullRequest, "url" | "title" | "number" | "body"> & { owner: string; name: string };
@@ -14,6 +15,23 @@ export type IssuesClosedByThisPr = {
   };
 };
 
+type LinkedIssue = {
+  number: number;
+  title: string;
+  state: string;
+  url: string;
+};
+
+type QueryResponse = {
+  repository: {
+    pullRequest: {
+      closingIssuesReferences: {
+        nodes: LinkedIssue[];
+      };
+    };
+  };
+};
+
 export const closedByPullRequestsReferences = /* GraphQL */ `
   query closingIssuesReferencesQuery($owner: String!, $repo: String!, $pr_number: Int!) {
     repository(owner: $owner, name: $repo) {
@@ -38,3 +56,32 @@ export const closedByPullRequestsReferences = /* GraphQL */ `
     }
   }
 `;
+
+export const linkedIssuesOnPullRequestReferences = /* GraphQL */ `
+  query ($owner: String!, $repo: String!, $prNumber: Int!) {
+    repository(owner: $owner, name: $repo) {
+      pullRequest(number: $prNumber) {
+        closingIssuesReferences(first: 100) {
+          nodes {
+            number
+            title
+            state
+            url
+          }
+        }
+      }
+    }
+  }
+`;
+
+export async function getLinkedIssues(context: Context) {
+  const prNumber = context.payload.pull_request.number;
+
+  const response = await context.octokit.graphql<QueryResponse>(linkedIssuesOnPullRequestReferences, {
+    owner: context.payload.repository.owner.login,
+    repo: context.payload.repository.name,
+    prNumber,
+  });
+
+  return response.repository.pullRequest.closingIssuesReferences.nodes;
+}

src/types/context.ts

@@ -3,7 +3,7 @@ import { PluginSettings } from "./plugin-input";
 import { Env } from "./env";
 import { createAdapters } from "../adapters";
 
-export type SupportedEvents = "pull_request.opened" | "pull_request.ready_for_review";
+export type SupportedEvents = "pull_request.opened" | "pull_request.ready_for_review" | "pull_request.edited";
 
 export type Context<TEvents extends SupportedEvents = SupportedEvents> = PluginContext<PluginSettings, Env, null, TEvents> & {
   adapters: ReturnType<typeof createAdapters>;

tests/issue-parsing.test.ts

@@ -0,0 +1,33 @@
+import { extractIssueUrls } from "../src/handlers/closing-keyword";
+import { expect, it, describe } from "@jest/globals";
+
+describe("extractIssueUrls", () => {
+  const defaultRepo = "owner/repo";
+
+  it("should remove duplicate issue references", () => {
+    const input = "Fixes #101 Resolves #101 Closes #101";
+    const result = extractIssueUrls(input, defaultRepo);
+
+    expect(result).toEqual(new Set([`https://github.com/${defaultRepo}/issues/101`]));
+  });
+
+  it("should handle mixed issue references", () => {
+    const input = `
+      Fixes https://github.com/owner/repo/issues/202
+      Closes owner/repo#303
+      Resolves #404
+    `;
+    const result = extractIssueUrls(input, defaultRepo);
+
+    expect(result).toEqual(
+      new Set(["https://github.com/owner/repo/issues/202", "https://github.com/owner/repo/issues/303", `https://github.com/${defaultRepo}/issues/404`])
+    );
+  });
+
+  it("should return an empty set for no matches", () => {
+    const input = "This is just a random text with no issues mentioned.";
+    const result = extractIssueUrls(input, defaultRepo);
+
+    expect(result).toEqual(new Set());
+  });
+});

tests/main.test.ts

@@ -184,28 +184,20 @@ describe("Pull Reviewer tests", () => {
     });
   });
 
-  it("should convert PR to draft if no issue is linked", async () => {
+  it("should skip precheck if no issue is linked", async () => {
     const { PullReviewer } = await import("../src/handlers/pull-reviewer");
     const context = createContext();
     const pullReviewer = new PullReviewer(context);
+    jest.spyOn(pullReviewer, "canPerformReview").mockImplementation(async () => true);
 
     // Mock empty closing issues
     jest.spyOn(pullReviewer, "checkIfPrClosesIssues").mockResolvedValue({
       closesIssues: false,
       issues: [],
     });
 
-    await expect(pullReviewer.getTaskNumberFromPullRequest(context)).rejects.toMatchObject({
-      logMessage: {
-        diff: "```diff\n! You need to link an issue before converting the pull request to ready for review.\n```",
-        level: "error",
-        raw: "You need to link an issue before converting the pull request to ready for review.",
-        type: "error",
-      },
-      metadata: {
-        caller: "PullReviewer.error",
-      },
-    });
+    expect(await pullReviewer.getTaskNumberFromPullRequest(context)).toBe(null);
+    expect(await pullReviewer.performPullPrecheck()).toEqual({ status: 200, reason: "Pull review data not found, Skipping automated review" });
   });
 });