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,63 @@
+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 oldMatch = extractIssueUrls(oldBody, context.payload.repository.full_name);
+  const newMatch = extractIssueUrls(newBody, context.payload.repository.full_name);
+
+  logger.info("Pull request body edit detected", {
+    oldClosingKeyword: oldMatch ? oldMatch[0] : null,
+    newClosingKeyword: newMatch ? newMatch[0] : null,
+  });
+
+  if (newMatch.length !== oldMatch.length || newMatch.some((url) => !oldMatch.includes(url))) {
+    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" };
+}
+
+function extractIssueUrls(pullBody: string, defaultRepo: string) {
+  const pattern =
+    /(?:close|closes|closed|fix|fixes|fixed|resolve|resolves|resolved)\s+(?:(https:\/\/github\.com\/([^/]+\/[^/]+)\/issues\/(\d+))|([^/\s]+\/[^#\s]+)#(\d+)|#(\d+))/gi;
+  const matches = pullBody.matchAll(pattern);
+  const issueUrls = [];
+
+  for (const match of matches) {
+    const fullUrl = match[1];
+    const repoPath = match[4];
+    const issueNum1 = match[5];
+    const issueNum2 = match[6];
+
+    if (fullUrl) {
+      issueUrls.push(fullUrl);
+    } else if (repoPath) {
+      issueUrls.push(`https://github.com/${repoPath}/issues/${issueNum1}`);
+    } else {
+      issueUrls.push(`https://github.com/${defaultRepo}/issues/${issueNum2}`);
+    }
+  }
+
+  return issueUrls.sort();
+}

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/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/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" });
   });
 });