fix: align API review "ready on" date with draft-aware open time

The API review check previously calculated its "ready on" date from
`pr.created_at`, while the actual merge-gating logic (24-hour rule)
used `getPROpenedTime` which accounts for draft→ready transitions.
This caused PRs that were opened as drafts to display a stale
"ready on" date.

Extract `getPROpenedTime` into a shared utility and use it in
`getPRReadyDate` so both the check display and the enforcement
use the same draft-aware timestamp.

Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>

Author: erickzhao

spec/api-review-state.spec.ts

@@ -62,6 +62,7 @@ describe('api review', () => {
           removeLabel: vi.fn().mockReturnValue({ data: [] }),
           listLabelsOnIssue: vi.fn().mockReturnValue({ data: [] }),
           listComments: vi.fn().mockReturnValue({ data: [] }),
+          listEventsForTimeline: vi.fn().mockReturnValue({ data: [] }),
         },
         checks: {
           listForRef: vi.fn().mockReturnValue({ data: { check_runs: [] } }),
@@ -108,9 +109,11 @@ describe('api review', () => {
     // Set created_at to yesterday.
     pull_request.created_at = new Date(+new Date() - 1000 * 60 * 60 * 24 * 2);
 
+    moctokit.rest.issues.listEventsForTimeline = vi.fn().mockReturnValue({ data: [] });
+
     const expectedTime = pull_request.created_at.getTime() + MINIMUM_MINOR_OPEN_TIME;
     const expectedDate = new Date(expectedTime).toISOString().split('T')[0];
-    const readyDate = getPRReadyDate(pull_request);
+    const readyDate = await getPRReadyDate(moctokit, pull_request);
 
     expect(readyDate).toEqual(expectedDate);
   });
@@ -121,9 +124,11 @@ describe('api review', () => {
     // Set created_at to yesterday.
     payload.created_at = new Date(+new Date() - 1000 * 60 * 60 * 24 * 2);
 
+    moctokit.rest.issues.listEventsForTimeline = vi.fn().mockReturnValue({ data: [] });
+
     const expectedTime = payload.created_at.getTime() + MINIMUM_PATCH_OPEN_TIME;
     const expectedDate = new Date(expectedTime).toISOString().split('T')[0];
-    const readyDate = getPRReadyDate(payload);
+    const readyDate = await getPRReadyDate(moctokit, payload);
 
     expect(readyDate).toEqual(expectedDate);
   });
@@ -134,8 +139,33 @@ describe('api review', () => {
     // Set created_at to yesterday.
     payload.created_at = new Date(+new Date() - 1000 * 60 * 60 * 24 * 2);
 
+    moctokit.rest.issues.listEventsForTimeline = vi.fn().mockReturnValue({ data: [] });
+
     const expectedDate = payload.created_at.toISOString().split('T')[0];
-    const readyDate = getPRReadyDate(payload);
+    const readyDate = await getPRReadyDate(moctokit, payload);
+
+    expect(readyDate).toEqual(expectedDate);
+  });
+
+  it('correctly returns PR ready date based on ready_for_review event when PR was a draft', async () => {
+    const { pull_request } = loadFixture('api-review-state/pull_request.semver-minor.json');
+
+    // PR was created 10 days ago but marked ready 2 days ago.
+    pull_request.created_at = new Date(+new Date() - 1000 * 60 * 60 * 24 * 10).toISOString();
+    const readyForReviewTime = new Date(+new Date() - 1000 * 60 * 60 * 24 * 2);
+
+    moctokit.rest.issues.listEventsForTimeline = vi.fn().mockReturnValue({
+      data: [
+        {
+          event: 'ready_for_review',
+          created_at: readyForReviewTime.toISOString(),
+        },
+      ],
+    });
+
+    const expectedTime = readyForReviewTime.getTime() + MINIMUM_MINOR_OPEN_TIME;
+    const expectedDate = new Date(expectedTime).toISOString().split('T')[0];
+    const readyDate = await getPRReadyDate(moctokit, pull_request);
 
     expect(readyDate).toEqual(expectedDate);
   });
@@ -531,6 +561,10 @@ describe('api review', () => {
           .get(`/repos/electron/electron/issues/${pull_request.number}/comments`)
           .reply(200, [c1]);
 
+        nock(GH_API)
+          .get(`/repos/electron/electron/issues/${pull_request.number}/timeline`)
+          .reply(200, []);
+
         nock(GH_API)
           .post('/repos/electron/electron/check-runs', (body) => {
             expect(body).toMatchObject({
@@ -584,6 +618,10 @@ describe('api review', () => {
           .get(`/repos/electron/electron/issues/${pull_request.number}/comments`)
           .reply(200, []);
 
+        nock(GH_API)
+          .get(`/repos/electron/electron/issues/${pull_request.number}/timeline`)
+          .reply(200, []);
+
         nock(GH_API)
           .get(`/repos/electron/electron/issues/${pull_request.number}/labels?per_page=100&page=1`)
           .reply(200, [
@@ -678,6 +716,10 @@ describe('api review', () => {
           .get(`/repos/electron/electron/issues/${pull_request.number}/comments`)
           .reply(200, []);
 
+        nock(GH_API)
+          .get(`/repos/electron/electron/issues/${pull_request.number}/timeline`)
+          .reply(200, []);
+
         nock(GH_API)
           .get(`/repos/electron/electron/issues/${pull_request.number}/labels?per_page=100&page=1`)
           .reply(200, [

spec/fixtures/api-review-state/pull_request.api-skip-delay_label.json

@@ -35,6 +35,18 @@
   "review_comment_url": "https://api.github.com/repos/electron/electron/pulls/comments{/number}",
   "comments_url": "https://api.github.com/repos/electron/electron/issues/26876/comments",
   "statuses_url": "https://api.github.com/repos/electron/electron/statuses/c6b1b7168ab850a47f856c4a30f7a441bede1117",
+  "base": {
+    "ref": "main",
+    "repo": {
+      "id": 9384267,
+      "name": "electron",
+      "full_name": "electron/electron",
+      "owner": {
+        "login": "electron"
+      },
+      "default_branch": "main"
+    }
+  },
   "head": {
     "label": "electron:fix-lint-js",
     "ref": "fix-lint-js",

spec/fixtures/api-review-state/pull_request.requested_review_label.json

@@ -45,6 +45,7 @@
     ],
     "base": {
       "repo": {
+        "full_name": "electron/electron",
         "owner": {
           "login": "electron"
         }

spec/fixtures/api-review-state/pull_request.semver-patch.json

@@ -35,6 +35,18 @@
   "review_comment_url": "https://api.github.com/repos/electron/electron/pulls/comments{/number}",
   "comments_url": "https://api.github.com/repos/electron/electron/issues/26876/comments",
   "statuses_url": "https://api.github.com/repos/electron/electron/statuses/c6b1b7168ab850a47f856c4a30f7a441bede1117",
+  "base": {
+    "ref": "main",
+    "repo": {
+      "id": 9384267,
+      "name": "electron",
+      "full_name": "electron/electron",
+      "owner": {
+        "login": "electron"
+      },
+      "default_branch": "main"
+    }
+  },
   "head": {
     "label": "electron:fix-lint-js",
     "ref": "fix-lint-js",

src/24-hour-rule.ts

@@ -1,4 +1,4 @@
-import { Context, Probot, ProbotOctokit } from 'probot';
+import { Context, Probot } from 'probot';
 
 import {
   NEW_PR_LABEL,
@@ -16,6 +16,9 @@ import { log } from './utils/log-util';
 import { addLabels, removeLabel } from './utils/label-utils';
 import { LogLevel } from './enums';
 import { PullRequest, Label, PullRequestLabeledEvent } from './types';
+import { getPROpenedTime } from './utils/pr-open-time-util';
+
+export { getPROpenedTime };
 
 const CHECK_INTERVAL = 1000 * 60 * 5;
 
@@ -36,40 +39,6 @@ export const getMinimumOpenTime = (pr: PullRequest): number => {
   return MINIMUM_MAJOR_OPEN_TIME;
 };
 
-/**
- * @param octokit - An Octokit instance
- * @returns a number representing the that cation should use as the
- * open time for the PR in milliseconds, taking draft status into account.
- */
-export const getPROpenedTime = async (octokit: ProbotOctokit, pr: PullRequest): Promise<number> => {
-  const [owner, repo] = pr.base.repo.full_name.split('/');
-
-  // Fetch PR timeline events.
-  const { data: events } = await octokit.rest.issues.listEventsForTimeline({
-    owner,
-    repo,
-    issue_number: pr.number,
-  });
-
-  // Filter out all except 'Ready For Review' events.
-  const readyForReviewEvents = events
-    .filter((e) => e.event === 'ready_for_review')
-    .sort((cA, cB) => {
-      if (!('created_at' in cA) || !('created_at' in cB)) return 0;
-      return new Date(cB.created_at).getTime() - new Date(cA.created_at).getTime();
-    });
-
-  // If this PR was a draft PR previously, set its opened time as a function
-  // of when it was most recently marked ready for review instead of when it was opened,
-  // otherwise return the PR open date.
-  const firstEvent = readyForReviewEvents[0];
-  const validFirstEvent = firstEvent && 'created_at' in firstEvent;
-
-  return validFirstEvent
-    ? new Date(firstEvent.created_at).getTime()
-    : new Date(pr.created_at).getTime();
-};
-
 export const shouldPRHaveLabel = async (
   github: Context['octokit'],
   pr: PullRequest,

src/api-review-state.ts

@@ -20,6 +20,7 @@ import { getEnvVar } from './utils/env-util';
 import { PullRequest, Label } from './types';
 import { GetResponseDataTypeFromEndpointMethod, Endpoints } from '@octokit/types';
 import { addLabels, removeLabel } from './utils/label-utils';
+import { getPROpenedTime } from './utils/pr-open-time-util';
 
 type APIApprovalState =
   ReturnType<typeof addOrUpdateAPIReviewCheck> extends Promise<infer T> ? T : unknown;
@@ -45,8 +46,8 @@ export const hasAPIReviewRequestedLabel = (pr: PullRequest) =>
  * @returns a date corresponding to the time that must elapse before a PR requiring
  *          API review is ready to be merged according to its semver label.
  */
-export const getPRReadyDate = (pr: PullRequest) => {
-  let readyTime = new Date(pr.created_at).getTime();
+export const getPRReadyDate = async (octokit: Context['octokit'], pr: PullRequest) => {
+  let readyTime = await getPROpenedTime(octokit, pr);
 
   if (pr.labels.some((l) => l.name === API_SKIP_DELAY_LABEL)) {
     log(
@@ -319,7 +320,7 @@ export async function addOrUpdateAPIReviewCheck(octokit: Context['octokit'], pr:
       output: {
         title: `${checkTitles[currentReviewLabel.name]} (${
           users.approved.length
-        }/2 LGTMs - ready on ${getPRReadyDate(pr)})`,
+        }/2 LGTMs - ready on ${await getPRReadyDate(octokit, pr)})`,
         summary: checkSummary,
       },
     });

src/utils/pr-open-time-util.ts

@@ -0,0 +1,31 @@
+import { ProbotOctokit } from 'probot';
+import { PullRequest } from '../types';
+
+/**
+ * @param octokit - An Octokit instance
+ * @returns a number representing the time that cation should use as the
+ * open time for the PR in milliseconds, taking draft status into account.
+ */
+export const getPROpenedTime = async (octokit: ProbotOctokit, pr: PullRequest): Promise<number> => {
+  const [owner, repo] = pr.base.repo.full_name.split('/');
+
+  const { data: events } = await octokit.rest.issues.listEventsForTimeline({
+    owner,
+    repo,
+    issue_number: pr.number,
+  });
+
+  const readyForReviewEvents = events
+    .filter((e) => e.event === 'ready_for_review')
+    .sort((cA, cB) => {
+      if (!('created_at' in cA) || !('created_at' in cB)) return 0;
+      return new Date(cB.created_at).getTime() - new Date(cA.created_at).getTime();
+    });
+
+  const firstEvent = readyForReviewEvents[0];
+  const validFirstEvent = firstEvent && 'created_at' in firstEvent;
+
+  return validFirstEvent
+    ? new Date(firstEvent.created_at).getTime()
+    : new Date(pr.created_at).getTime();
+};