From b2963b9127309fd8ef897bb24a5d79761d566111 Mon Sep 17 00:00:00 2001 From: Ashwin Bhat Date: Mon, 3 Aug 2026 19:15:50 -0700 Subject: [PATCH] Derive trigger timestamps for issues and pull_request events (#1592) * Derive trigger timestamps for issues and pull_request events For issues labeled/assigned triggers, look up the matching event in the issue's event history to get the exact time of the label/assignment, falling back to the payload's updated_at/created_at when the lookup fails. issues opened uses issue.created_at; pull_request opened uses pull_request.created_at and other pull_request actions use updated_at. * Ignore issue label/assign events older than the payload snapshot A matching labeled/assigned event that predates the webhook payload's issue.updated_at cannot be the event that fired the webhook, so fall back to the payload timestamps instead of adopting it as the boundary. --- src/github/data/fetcher.ts | 134 +++++++++++++ src/modes/tag/index.ts | 4 +- test/data-fetcher.test.ts | 380 ++++++++++++++++++++++++++++++++++++- test/mockContext.ts | 8 +- 4 files changed, 520 insertions(+), 6 deletions(-) diff --git a/src/github/data/fetcher.ts b/src/github/data/fetcher.ts index cea2ea24..8aad4ec4 100644 --- a/src/github/data/fetcher.ts +++ b/src/github/data/fetcher.ts @@ -1,4 +1,5 @@ import { execFileSync } from "child_process"; +import type { IssuesEvent } from "@octokit/webhooks-types"; import type { Octokits } from "../api/client"; import { ISSUE_QUERY, PR_QUERY, USER_QUERY } from "../api/queries/github"; import { @@ -29,6 +30,12 @@ import { * Extracts the trigger timestamp from the GitHub webhook payload. * This timestamp represents when the triggering comment/review/event was created. * + * For `issues` and `pull_request` events there is no dedicated trigger + * object in the payload, so the issue/PR's own timestamps from the webhook + * snapshot are used: `created_at` for opened events, otherwise `updated_at` + * (falling back to `created_at`). For issues labeled/assigned events, + * prefer resolveTriggerTimestamp() which looks up the exact event time. + * * @param context - Parsed GitHub context from webhook * @returns ISO timestamp string or undefined if not available */ @@ -41,11 +48,138 @@ export function extractTriggerTimestamp( return context.payload.review.submitted_at || undefined; } else if (isPullRequestReviewCommentEvent(context)) { return context.payload.comment.created_at || undefined; + } else if (isIssuesEvent(context)) { + const issue = context.payload.issue; + if (context.eventAction === "opened") { + return issue?.created_at || issue?.updated_at || undefined; + } + // updated_at reflects the last comment or edit on the issue, so the + // newest pre-existing comment can share this timestamp and be excluded + // along with anything newer. + return issue?.updated_at || issue?.created_at || undefined; + } else if (isPullRequestEvent(context)) { + const pullRequest = context.payload.pull_request; + if (context.eventAction === "opened") { + return pullRequest?.created_at || pullRequest?.updated_at || undefined; + } + return pullRequest?.updated_at || pullRequest?.created_at || undefined; } return undefined; } +/** + * Resolves the trigger timestamp for the event, consulting the GitHub API + * where the webhook payload does not carry an exact time for the triggering + * action. + * + * For issues labeled/assigned events the label/assignment carries no + * timestamp of its own in the payload, so the matching entry in the issue's + * event history is looked up and its `created_at` is used. If the lookup + * fails, this falls back to extractTriggerTimestamp(). + * + * @param context - Parsed GitHub context from webhook + * @param octokits - GitHub API clients + * @returns ISO timestamp string or undefined if not available + */ +export async function resolveTriggerTimestamp( + context: ParsedGitHubContext, + octokits: Octokits, +): Promise { + if ( + isIssuesEvent(context) && + (context.eventAction === "labeled" || context.eventAction === "assigned") + ) { + const eventTime = await findIssueEventTime(context, octokits); + if (eventTime) { + return eventTime; + } + console.warn( + `Could not resolve the ${context.eventAction} event time for issue #${context.entityNumber}; falling back to the webhook payload timestamps`, + ); + } + + return extractTriggerTimestamp(context); +} + +/** + * Looks up the most recent labeled/assigned event on the issue that matches + * the label or assignee in the webhook payload, returning its created_at. + */ +async function findIssueEventTime( + context: ParsedGitHubContext & { payload: IssuesEvent }, + octokits: Octokits, +): Promise { + const payload = context.payload; + let matches: (event: { + event: string; + label?: { name?: string | null }; + assignee?: { login?: string } | null; + }) => boolean; + + if (payload.action === "labeled") { + const labelName = payload.label?.name; + if (!labelName) return undefined; + matches = (event) => + event.event === "labeled" && event.label?.name === labelName; + } else if (payload.action === "assigned") { + const assigneeLogin = payload.assignee?.login; + if (!assigneeLogin) return undefined; + matches = (event) => + event.event === "assigned" && event.assignee?.login === assigneeLogin; + } else { + return undefined; + } + + try { + const events = await octokits.rest.paginate( + octokits.rest.issues.listEvents, + { + owner: context.repository.owner, + repo: context.repository.repo, + issue_number: context.entityNumber, + per_page: 100, + }, + ); + + let latest: (typeof events)[number] | undefined; + for (const event of events.filter(matches)) { + if ( + !latest || + new Date(event.created_at).getTime() > + new Date(latest.created_at).getTime() + ) { + latest = event; + } + } + + // Labeling/assignment does not bump the issue's updated_at, so the event + // that fired this webhook cannot predate the payload snapshot's + // updated_at. An older match means the current event is not visible in + // the events API yet; ignore it rather than adopt a stale boundary. + const snapshotUpdatedAt = payload.issue?.updated_at; + if ( + latest && + snapshotUpdatedAt && + new Date(latest.created_at).getTime() < + new Date(snapshotUpdatedAt).getTime() + ) { + console.warn( + `Latest matching ${payload.action} event on issue #${context.entityNumber} predates the issue's updated_at; treating it as stale`, + ); + return undefined; + } + + return latest?.created_at || undefined; + } catch (error) { + console.warn( + `Failed to fetch events for issue #${context.entityNumber}:`, + error, + ); + return undefined; + } +} + /** * Extracts the original title from the GitHub webhook payload. * This is the title as it existed when the trigger event occurred. diff --git a/src/modes/tag/index.ts b/src/modes/tag/index.ts index bfbeaea5..c6de58dd 100644 --- a/src/modes/tag/index.ts +++ b/src/modes/tag/index.ts @@ -8,7 +8,7 @@ import { import { prepareMcpConfig } from "../../mcp/install-mcp-server"; import { fetchGitHubData, - extractTriggerTimestamp, + resolveTriggerTimestamp, extractOriginalTitle, extractOriginalBody, } from "../../github/data/fetcher"; @@ -45,7 +45,7 @@ export async function prepareTagMode({ const commentData = await createInitialComment(octokit.rest, context); const commentId = commentData.id; - const triggerTime = extractTriggerTimestamp(context); + const triggerTime = await resolveTriggerTimestamp(context, octokit); const originalTitle = extractOriginalTitle(context); const originalBody = extractOriginalBody(context); diff --git a/test/data-fetcher.test.ts b/test/data-fetcher.test.ts index c054039b..cc2af464 100644 --- a/test/data-fetcher.test.ts +++ b/test/data-fetcher.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it, jest, test } from "bun:test"; import { extractTriggerTimestamp, + resolveTriggerTimestamp, extractOriginalTitle, extractOriginalBody, fetchGitHubData, @@ -8,6 +9,7 @@ import { filterReviewsToTriggerTime, isBodySafeToUse, } from "../src/github/data/fetcher"; +import type { ParsedGitHubContext } from "../src/github/context"; import { createMockContext, mockIssueCommentContext, @@ -16,6 +18,8 @@ import { mockPullRequestReviewCommentContext, mockPullRequestOpenedContext, mockIssueOpenedContext, + mockIssueAssignedContext, + mockIssueLabeledContext, } from "./mockContext"; import type { GitHubComment, GitHubReview } from "../src/github/types"; @@ -38,15 +42,112 @@ describe("extractTriggerTimestamp", () => { expect(timestamp).toBe("2024-01-15T16:45:00Z"); }); - it("should return undefined for pull_request event", () => { + it("should extract created_at timestamp from pull_request opened event", () => { const context = mockPullRequestOpenedContext; const timestamp = extractTriggerTimestamp(context); - expect(timestamp).toBeUndefined(); + expect(timestamp).toBe("2024-01-15T14:00:00Z"); }); - it("should return undefined for issues event", () => { + it("should extract updated_at timestamp from pull_request synchronize event", () => { + const context: ParsedGitHubContext = { + ...mockPullRequestOpenedContext, + eventAction: "synchronize", + payload: { + ...(mockPullRequestOpenedContext.payload as any), + action: "synchronize", + }, + }; + const timestamp = extractTriggerTimestamp(context); + expect(timestamp).toBe("2024-01-15T14:05:00Z"); + }); + + it("should extract updated_at timestamp from pull_request edited event", () => { + const context: ParsedGitHubContext = { + ...mockPullRequestOpenedContext, + eventAction: "edited", + payload: { + ...(mockPullRequestOpenedContext.payload as any), + action: "edited", + }, + }; + const timestamp = extractTriggerTimestamp(context); + expect(timestamp).toBe("2024-01-15T14:05:00Z"); + }); + + it("should extract created_at timestamp from issues opened event", () => { const context = mockIssueOpenedContext; const timestamp = extractTriggerTimestamp(context); + expect(timestamp).toBe("2024-01-15T10:30:00Z"); + }); + + it("should fall back to updated_at for issues labeled event", () => { + const context = mockIssueLabeledContext; + const timestamp = extractTriggerTimestamp(context); + expect(timestamp).toBe("2024-01-15T11:30:00Z"); + }); + + it("should fall back to updated_at for issues assigned event", () => { + const context = mockIssueAssignedContext; + const timestamp = extractTriggerTimestamp(context); + expect(timestamp).toBe("2024-01-15T11:00:00Z"); + }); + + it("should fall back to created_at for issues labeled event without updated_at", () => { + const context = createMockContext({ + eventName: "issues", + eventAction: "labeled", + payload: { + action: "labeled", + issue: { + number: 1, + title: "test", + body: "test", + created_at: "2024-01-15T08:00:00Z", + }, + } as any, + entityNumber: 1, + isPR: false, + }); + const timestamp = extractTriggerTimestamp(context); + expect(timestamp).toBe("2024-01-15T08:00:00Z"); + }); + + it("should fall back to created_at for pull_request synchronize event without updated_at", () => { + const context = createMockContext({ + eventName: "pull_request", + eventAction: "synchronize", + payload: { + action: "synchronize", + pull_request: { + number: 1, + title: "test", + body: "test", + created_at: "2024-01-15T08:30:00Z", + }, + } as any, + entityNumber: 1, + isPR: true, + }); + const timestamp = extractTriggerTimestamp(context); + expect(timestamp).toBe("2024-01-15T08:30:00Z"); + }); + + it("should return undefined for issues event without timestamps", () => { + const context = createMockContext({ + eventName: "issues", + eventAction: "labeled", + payload: { + action: "labeled", + issue: { + number: 1, + title: "test", + body: "test", + }, + } as any, + entityNumber: 1, + isPR: false, + }); + const timestamp = extractTriggerTimestamp(context); expect(timestamp).toBeUndefined(); }); @@ -66,6 +167,195 @@ describe("extractTriggerTimestamp", () => { }); }); +describe("resolveTriggerTimestamp", () => { + const createEventsOctokits = (events: any[]) => { + const paginate = jest.fn().mockResolvedValue(events); + return { + octokits: { + rest: { + paginate, + issues: { listEvents: jest.fn() }, + }, + graphql: jest.fn(), + } as any, + paginate, + }; + }; + + it("should use the labeled event time for issues labeled event", async () => { + const { octokits, paginate } = createEventsOctokits([ + { + event: "labeled", + label: { name: "other-label" }, + created_at: "2024-01-15T10:45:00Z", + }, + { + event: "labeled", + label: { name: "claude-task" }, + created_at: "2024-01-15T10:50:00Z", + }, + { + event: "labeled", + label: { name: "claude-task" }, + created_at: "2024-01-15T11:45:00Z", + }, + { + event: "assigned", + assignee: { login: "claude-bot" }, + created_at: "2024-01-15T11:50:00Z", + }, + ]); + + const timestamp = await resolveTriggerTimestamp( + mockIssueLabeledContext, + octokits, + ); + + expect(timestamp).toBe("2024-01-15T11:45:00Z"); + expect(paginate).toHaveBeenCalledWith(octokits.rest.issues.listEvents, { + owner: "test-owner", + repo: "test-repo", + issue_number: 1234, + per_page: 100, + }); + }); + + it("should use the assigned event time for issues assigned event", async () => { + const { octokits } = createEventsOctokits([ + { + event: "assigned", + assignee: { login: "someone-else" }, + created_at: "2024-01-15T10:40:00Z", + }, + { + event: "assigned", + assignee: { login: "claude-bot" }, + created_at: "2024-01-15T11:05:00Z", + }, + ]); + + const timestamp = await resolveTriggerTimestamp( + mockIssueAssignedContext, + octokits, + ); + + expect(timestamp).toBe("2024-01-15T11:05:00Z"); + }); + + it("should ignore a matching event that predates the issue's updated_at", async () => { + // A match older than the payload's updated_at is a previous + // labeling, not the one that fired this webhook. + const { octokits } = createEventsOctokits([ + { + event: "labeled", + label: { name: "claude-task" }, + created_at: "2024-01-15T10:50:00Z", + }, + ]); + + const timestamp = await resolveTriggerTimestamp( + mockIssueLabeledContext, + octokits, + ); + + expect(timestamp).toBe("2024-01-15T11:30:00Z"); + }); + + it("should fall back to updated_at when no matching labeled event exists", async () => { + const { octokits } = createEventsOctokits([ + { + event: "labeled", + label: { name: "unrelated" }, + created_at: "2024-01-15T10:45:00Z", + }, + ]); + + const timestamp = await resolveTriggerTimestamp( + mockIssueLabeledContext, + octokits, + ); + + expect(timestamp).toBe("2024-01-15T11:30:00Z"); + }); + + it("should fall back to updated_at when the events lookup fails", async () => { + const octokits = { + rest: { + paginate: jest.fn().mockRejectedValue(new Error("API failure")), + issues: { listEvents: jest.fn() }, + }, + graphql: jest.fn(), + } as any; + + const timestamp = await resolveTriggerTimestamp( + mockIssueAssignedContext, + octokits, + ); + + expect(timestamp).toBe("2024-01-15T11:00:00Z"); + }); + + it("should fall back to created_at when updated_at is also missing", async () => { + const context = createMockContext({ + eventName: "issues", + eventAction: "labeled", + payload: { + action: "labeled", + label: { name: "claude-task" }, + issue: { + number: 1, + title: "test", + body: "test", + created_at: "2024-01-15T08:00:00Z", + }, + } as any, + entityNumber: 1, + isPR: false, + }); + const { octokits } = createEventsOctokits([]); + + const timestamp = await resolveTriggerTimestamp(context, octokits); + + expect(timestamp).toBe("2024-01-15T08:00:00Z"); + }); + + it("should use created_at for issues opened event without an API call", async () => { + const { octokits, paginate } = createEventsOctokits([]); + + const timestamp = await resolveTriggerTimestamp( + mockIssueOpenedContext, + octokits, + ); + + expect(timestamp).toBe("2024-01-15T10:30:00Z"); + expect(paginate).not.toHaveBeenCalled(); + }); + + it("should use created_at for pull_request opened event without an API call", async () => { + const { octokits, paginate } = createEventsOctokits([]); + + const timestamp = await resolveTriggerTimestamp( + mockPullRequestOpenedContext, + octokits, + ); + + expect(timestamp).toBe("2024-01-15T14:00:00Z"); + expect(paginate).not.toHaveBeenCalled(); + }); + + it("should use the existing comment timestamp for issue_comment events", async () => { + const { octokits, paginate } = createEventsOctokits([]); + + const timestamp = await resolveTriggerTimestamp( + mockIssueCommentContext, + octokits, + ); + + expect(timestamp).toBe("2024-01-15T12:30:00Z"); + expect(paginate).not.toHaveBeenCalled(); + }); +}); + describe("extractOriginalTitle", () => { it("should extract title from IssueCommentEvent on PR", () => { const title = extractOriginalTitle(mockPullRequestCommentContext); @@ -659,6 +949,90 @@ describe("fetchGitHubData integration with time filtering", () => { expect(result.comments[0]?.body).toBe("Comment before trigger"); }); + it("should filter comments using the resolved issues labeled event time", async () => { + const mockOctokits = { + graphql: jest.fn().mockResolvedValue({ + repository: { + issue: { + number: 1234, + title: "Test Issue", + body: "Issue body", + author: { login: "author" }, + comments: { + nodes: [ + { + id: "1", + databaseId: "1", + body: "Comment before label", + author: { login: "user1" }, + createdAt: "2024-01-15T10:00:00Z", + updatedAt: "2024-01-15T10:00:00Z", + }, + { + id: "2", + databaseId: "2", + body: "Comment created after label", + author: { login: "user2" }, + createdAt: "2024-01-15T12:00:00Z", + updatedAt: "2024-01-15T12:00:00Z", + }, + { + id: "3", + databaseId: "3", + body: "Comment edited after label", + author: { login: "user3" }, + createdAt: "2024-01-15T10:00:00Z", + updatedAt: "2024-01-15T12:00:00Z", + lastEditedAt: "2024-01-15T12:00:00Z", + }, + { + id: "4", + databaseId: "4", + body: "Latest comment before label", + author: { login: "user4" }, + createdAt: "2024-01-15T11:30:00Z", + updatedAt: "2024-01-15T11:30:00Z", + }, + ], + }, + }, + }, + user: { login: "trigger-user" }, + }), + rest: { + paginate: jest.fn().mockResolvedValue([ + { + event: "labeled", + label: { name: "claude-task" }, + created_at: "2024-01-15T11:45:00Z", + }, + ]), + issues: { listEvents: jest.fn() }, + }, + }; + + // The issues (labeled) webhook has no trigger comment; the boundary is + // the labeled event's own timestamp from the issue event history. + const triggerTime = await resolveTriggerTimestamp( + mockIssueLabeledContext, + mockOctokits as any, + ); + expect(triggerTime).toBe("2024-01-15T11:45:00Z"); + + const result = await fetchGitHubData({ + octokits: mockOctokits as any, + repository: "test-owner/test-repo", + prNumber: "1234", + isPR: false, + triggerUsername: "trigger-user", + triggerTime, + }); + + // Comments created before the label are kept (including the most + // recent one); comments created or edited after it are excluded. + expect(result.comments.map((c) => c.id)).toEqual(["1", "4"]); + }); + it("should filter PR reviews based on trigger time", async () => { const mockOctokits = { graphql: jest.fn().mockResolvedValue({ diff --git a/test/mockContext.ts b/test/mockContext.ts index 324104e6..93b7ac0b 100644 --- a/test/mockContext.ts +++ b/test/mockContext.ts @@ -140,7 +140,7 @@ export const mockIssueOpenedContext: ParsedGitHubContext = { body: "## Description\n\nThe application crashes immediately after launching.\n\n## Steps to reproduce\n\n1. Install the app\n2. Launch it\n3. See crash\n\n/claude please help me fix this", assignee: null, created_at: "2024-01-15T10:30:00Z", - updated_at: "2024-01-15T10:30:00Z", + updated_at: "2024-01-15T10:35:00Z", html_url: "https://github.com/test-owner/test-repo/issues/42", user: { login: "john-doe", @@ -191,6 +191,8 @@ export const mockIssueAssignedContext: ParsedGitHubContext = { avatar_url: "https://avatars.githubusercontent.com/u/11111", html_url: "https://github.com/claude-bot", }, + created_at: "2024-01-15T09:00:00Z", + updated_at: "2024-01-15T11:00:00Z", }, repository: { name: "test-repo", @@ -225,6 +227,8 @@ export const mockIssueLabeledContext: ParsedGitHubContext = { html_url: "https://github.com/alice-wonder", }, assignee: null, + created_at: "2024-01-15T09:30:00Z", + updated_at: "2024-01-15T11:30:00Z", }, label: { id: 987654321, @@ -355,6 +359,8 @@ export const mockPullRequestOpenedContext: ParsedGitHubContext = { avatar_url: "https://avatars.githubusercontent.com/u/55555", html_url: "https://github.com/feature-developer", }, + created_at: "2024-01-15T14:00:00Z", + updated_at: "2024-01-15T14:05:00Z", }, repository: { name: "test-repo",