From 3129ef2842ce33b6644da45686a924fcc5efa29f Mon Sep 17 00:00:00 2001 From: Guillermo Casanova Date: Sat, 3 Oct 2026 22:16:04 -0300 Subject: [PATCH] fix(server): PR watch wakes the agent when a bot edits its review comment --- .../orchestration-v2/pullRequestWatch.test.ts | 22 +++++++++++- .../src/orchestration-v2/pullRequestWatch.ts | 10 +++--- .../src/pullRequest/GitHubPullRequestCli.ts | 3 ++ .../GitHubPullRequestProvider.test.ts | 3 ++ .../pullRequest/GitHubPullRequestProvider.ts | 2 ++ .../pullRequest/gitHubPullRequestJson.test.ts | 34 ++++++++++++++++++- .../src/pullRequest/gitHubPullRequestJson.ts | 23 ++++++++++--- packages/contracts/src/pullRequest.ts | 2 ++ 8 files changed, 89 insertions(+), 10 deletions(-) diff --git a/apps/server/src/orchestration-v2/pullRequestWatch.test.ts b/apps/server/src/orchestration-v2/pullRequestWatch.test.ts index c247d78fd332..05c1013b1c5f 100644 --- a/apps/server/src/orchestration-v2/pullRequestWatch.test.ts +++ b/apps/server/src/orchestration-v2/pullRequestWatch.test.ts @@ -1,7 +1,6 @@ import type { PullRequestCheck, PullRequestComment, - PullRequestDetail, ThreadPullRequestWatch, } from "@t3tools/contracts"; import { assert, describe, it } from "@effect/vitest"; @@ -119,6 +118,27 @@ describe("evaluatePullRequestWatch", () => { assert.deepEqual(again.next.remarkIds, [first.id, "late"]); }); + it("reports edits after the watermark once and counts them toward the wake limit", () => { + const old = remark("greptile[bot]", "2026-10-02T11:00:00Z"); + const watching = watch({ + headSha: "aaaaaaaaaa", + remarkIds: [old.id], + wakes: PULL_REQUEST_WATCH_WAKE_LIMIT - 1, + }); + assert.deepEqual(evaluatePullRequestWatch(watching, detail(), [old]).changes, []); + const edited = { ...old, editedAt: "2026-10-02T12:06:00Z" }; + const report = evaluatePullRequestWatch(watching, detail(), [edited]); + assert.deepEqual(report.changes, [{ kind: "remarks", remarks: [edited] }]); + assert.equal(report.next.remarksThrough, edited.editedAt); + assert.deepEqual(report.next.remarkIds, [old.id]); + assert.isTrue(report.exhausted); + assert.deepEqual(evaluatePullRequestWatch(report.next, detail(), [edited]).changes, []); + const late = { ...edited, id: "late" }; + assert.deepEqual(evaluatePullRequestWatch(report.next, detail(), [edited, late]).changes, [ + { kind: "remarks", remarks: [late] }, + ]); + }); + it("does not treat a failed check read as a rerun", () => { const failed = detail({ checks: [check("lint", "failure")] }); const reported = evaluatePullRequestWatch(watch(), failed, noRemarks); diff --git a/apps/server/src/orchestration-v2/pullRequestWatch.ts b/apps/server/src/orchestration-v2/pullRequestWatch.ts index af9ce27636ee..4a0c2ec3197a 100644 --- a/apps/server/src/orchestration-v2/pullRequestWatch.ts +++ b/apps/server/src/orchestration-v2/pullRequestWatch.ts @@ -71,18 +71,20 @@ export function evaluatePullRequestWatch( const own = (detail.viewer ?? detail.author?.login)?.toLowerCase(); const through = Date.parse(watch.remarksThrough); + // An edit counts as new activity, so bots that rewrite one summary comment still wake the agent. + const activeAt = (remark: PullRequestComment) => remark.editedAt ?? remark.createdAt; // GitHub times are per second, so remarks at the boundary time are told apart by ID. const fresh = (remarks ?? []).filter((remark) => { - const at = Date.parse(remark.createdAt); + const at = Date.parse(activeAt(remark)); return ( (at > through || (at === through && !watch.remarkIds.includes(remark.id))) && remark.author?.login.toLowerCase() !== own ); }); if (fresh.length > 0) changes.push({ kind: "remarks", remarks: fresh }); - const latest = Math.max(through, ...fresh.map((remark) => Date.parse(remark.createdAt))); - const atLatest = fresh.filter((remark) => Date.parse(remark.createdAt) === latest); - const remarksThrough = latest === through ? watch.remarksThrough : atLatest[0]!.createdAt; + const latest = Math.max(through, ...fresh.map((remark) => Date.parse(activeAt(remark)))); + const atLatest = fresh.filter((remark) => Date.parse(activeAt(remark)) === latest); + const remarksThrough = latest === through ? watch.remarksThrough : activeAt(atLatest[0]!); const remarkIds = [ ...(latest === through ? watch.remarkIds : []), ...atLatest.map((remark) => remark.id), diff --git a/apps/server/src/pullRequest/GitHubPullRequestCli.ts b/apps/server/src/pullRequest/GitHubPullRequestCli.ts index f40cff8f50bf..3bf68619ae42 100644 --- a/apps/server/src/pullRequest/GitHubPullRequestCli.ts +++ b/apps/server/src/pullRequest/GitHubPullRequestCli.ts @@ -2338,6 +2338,7 @@ export const make = Effect.gen(function* () { let reviewers: ReadonlyArray = []; let reactions: GitHubReviewThreadPage["reactions"] = []; const reactionsById = new Map>(); + const editedAtById = new Map(); let commits: GitHubReviewThreadPage["commits"] = []; let viewer: GitHubReviewThreadPage["viewer"] = { canUpdate: true, didAuthor: false }; const dismissalsByReviewId = new Map(); @@ -2356,6 +2357,7 @@ export const make = Effect.gen(function* () { reviewers = read.reviewers; reactions = read.reactions; for (const [id, entry] of read.reactionsById) reactionsById.set(id, entry); + for (const [id, editedAt] of read.editedAtById) editedAtById.set(id, editedAt); commits = read.commits; viewer = read.viewer; for (const [id, message] of read.dismissalsByReviewId) @@ -2411,6 +2413,7 @@ export const make = Effect.gen(function* () { truncated: cursor !== null || entries.some((entry) => entry.nextCommentCursor !== null), reactions, reactionsById, + editedAtById, reviewers, avatarsByLogin, botLogins, diff --git a/apps/server/src/pullRequest/GitHubPullRequestProvider.test.ts b/apps/server/src/pullRequest/GitHubPullRequestProvider.test.ts index 4b2d618a0768..8e3b97b2a6fe 100644 --- a/apps/server/src/pullRequest/GitHubPullRequestProvider.test.ts +++ b/apps/server/src/pullRequest/GitHubPullRequestProvider.test.ts @@ -884,6 +884,7 @@ describe("getChangeRequest commits", () => { truncated: false, reactions: [], reactionsById: new Map>(), + editedAtById: new Map(), reviewers: [], avatarsByLogin: new Map(), botLogins: new Set(), @@ -969,6 +970,7 @@ describe("getChangeRequestActivity dismissed reviews", () => { truncated: false, reactions: [], reactionsById: new Map(), + editedAtById: new Map([["PRR_1", "2026-07-04T00:00:00Z"]]), reviewers: [], avatarsByLogin: new Map(), botLogins: new Set(["macroscopeapp"]), @@ -1000,6 +1002,7 @@ describe("getChangeRequestActivity dismissed reviews", () => { Effect.map((activity) => { expect(activity.comments[0]?.body).toBe("Dismissing prior approval to re-evaluate 9b66581"); expect(activity.comments[0]?.author?.isBot).toBe(true); + expect(activity.comments[0]?.editedAt).toBe("2026-07-04T00:00:00Z"); }), Effect.provide(layerFor("")), ), diff --git a/apps/server/src/pullRequest/GitHubPullRequestProvider.ts b/apps/server/src/pullRequest/GitHubPullRequestProvider.ts index 5e70c2ee1ed4..c557fd9fc3e0 100644 --- a/apps/server/src/pullRequest/GitHubPullRequestProvider.ts +++ b/apps/server/src/pullRequest/GitHubPullRequestProvider.ts @@ -408,6 +408,7 @@ export const make = Effect.gen(function* () { dismissalsByReviewId: new Map(), reactions: [], reactionsById: new Map>(), + editedAtById: new Map(), reviewThreads: [], commentCount: 0, truncated: true, @@ -473,6 +474,7 @@ export const make = Effect.gen(function* () { // A comment out of `gh pr view --json` carries none of its own: that read // reports no reaction at all, so they arrive from the GraphQL page by node id. reactions: comment.reactions ?? reviewThreads.reactionsById.get(comment.id) ?? [], + editedAt: reviewThreads.editedAtById.get(comment.id) ?? comment.editedAt ?? null, })) .toSorted((left, right) => left.createdAt.localeCompare(right.createdAt)), // `gh pr view --json comments,reviews` follows GitHub's cursors itself, so those two diff --git a/apps/server/src/pullRequest/gitHubPullRequestJson.test.ts b/apps/server/src/pullRequest/gitHubPullRequestJson.test.ts index 1393ab6e6b6a..c564e82918d4 100644 --- a/apps/server/src/pullRequest/gitHubPullRequestJson.test.ts +++ b/apps/server/src/pullRequest/gitHubPullRequestJson.test.ts @@ -1105,7 +1105,13 @@ describe("review thread decoding", () => { path: "src/a.ts", line: 42, diffSide: "LEFT", - comments: { totalCount: 2, nodes: [comment("c1", "first"), comment("c2", "second")] }, + comments: { + totalCount: 2, + nodes: [ + { ...comment("c1", "first"), lastEditedAt: "2026-07-02T00:00:00Z" }, + comment("c2", "second"), + ], + }, }, ]), ), @@ -1124,6 +1130,7 @@ describe("review thread decoding", () => { author: { login: "bilal", name: null, avatarUrl: "https://avatars/b.png" }, body: "first", createdAt: "2026-07-01T00:00:00Z", + editedAt: "2026-07-02T00:00:00Z", url: "https://github.com/acme/web/pull/1#discussion_rc1", reactions: [], }, @@ -1132,12 +1139,16 @@ describe("review thread decoding", () => { author: { login: "bilal", name: null, avatarUrl: "https://avatars/b.png" }, body: "second", createdAt: "2026-07-01T00:00:00Z", + editedAt: null, url: "https://github.com/acme/web/pull/1#discussion_rc2", reactions: [], }, ], }, ]); + expect( + reviewThreadConversation(reviewThreads.threads.map((entry) => entry.thread))[0]?.editedAt, + ).toBe("2026-07-02T00:00:00Z"); }); it("leaves an outdated thread without a line rather than pinning it to a stale one", () => { @@ -1186,6 +1197,27 @@ describe("review thread decoding", () => { expect(threads).toHaveLength(1); }); + it("reads edit times for issue comments and reviews without using reaction updates", () => { + const editedAt = "2026-10-02T12:06:00Z"; + const result = expectSuccess( + decodeReviewThreadsJson( + threadsJson([], { + comments: { + nodes: [ + { id: "edited", lastEditedAt: editedAt }, + { id: "reaction-only", lastEditedAt: null, updatedAt: editedAt }, + ], + }, + reviews: { nodes: [{ id: "review", lastEditedAt: editedAt }] }, + }), + ), + ); + expect([...result.editedAtById]).toEqual([ + ["edited", editedAt], + ["review", editedAt], + ]); + }); + it("puts an issue comment's and a review's reactions in reactionsById, and the pull request's own in reactions", () => { const result = expectSuccess( decodeReviewThreadsJson( diff --git a/apps/server/src/pullRequest/gitHubPullRequestJson.ts b/apps/server/src/pullRequest/gitHubPullRequestJson.ts index d1600754b50b..b733ff46d703 100644 --- a/apps/server/src/pullRequest/gitHubPullRequestJson.ts +++ b/apps/server/src/pullRequest/gitHubPullRequestJson.ts @@ -362,6 +362,7 @@ function toReactions( const RawCommentSchema = Schema.Struct({ id: Schema.String, + lastEditedAt: Schema.optional(Schema.NullOr(Schema.String)), author: Schema.optional(Schema.NullOr(RawActorSchema)), body: Schema.optional(Schema.String), createdAt: Schema.String, @@ -372,6 +373,7 @@ const RawCommentSchema = Schema.Struct({ const RawReviewSchema = Schema.Struct({ id: Schema.String, + lastEditedAt: Schema.optional(Schema.NullOr(Schema.String)), author: Schema.optional(Schema.NullOr(RawActorSchema)), body: Schema.optional(Schema.String), state: Schema.optional(Schema.NullOr(Schema.String)), @@ -490,6 +492,7 @@ const RawReviewThreadsSchema = Schema.Struct({ Schema.Struct({ id: Schema.optional(Schema.NullOr(Schema.String)), author: Schema.optional(Schema.NullOr(RawActorSchema)), + lastEditedAt: Schema.optional(Schema.NullOr(Schema.String)), reactionGroups: RawReactionGroupsSchema, }), ), @@ -507,6 +510,7 @@ const RawReviewThreadsSchema = Schema.Struct({ Schema.Struct({ id: Schema.optional(Schema.NullOr(Schema.String)), author: Schema.optional(Schema.NullOr(RawActorSchema)), + lastEditedAt: Schema.optional(Schema.NullOr(Schema.String)), reactionGroups: RawReactionGroupsSchema, }), ), @@ -885,7 +889,7 @@ export const REVIEW_THREADS_GRAPHQL_QUERY = `query($owner: String!, $name: Strin comments(first: 10) { totalCount pageInfo { hasNextPage endCursor } - nodes { id author { __typename login avatarUrl } body createdAt url ${REACTION_GROUPS_FIELDS} } + nodes { id author { __typename login avatarUrl } body createdAt lastEditedAt url ${REACTION_GROUPS_FIELDS} } } } } @@ -894,9 +898,9 @@ export const REVIEW_THREADS_GRAPHQL_QUERY = `query($owner: String!, $name: Strin author { __typename login avatarUrl } ${REACTION_GROUPS_FIELDS} comments(first: ${GRAPHQL_PAGE_SIZE}) { - nodes { id author { __typename login avatarUrl } ${REACTION_GROUPS_FIELDS} } + nodes { id lastEditedAt author { __typename login avatarUrl } ${REACTION_GROUPS_FIELDS} } } - reviews(first: ${GRAPHQL_PAGE_SIZE}) { nodes { id author { __typename login avatarUrl } ${REACTION_GROUPS_FIELDS} } } + reviews(first: ${GRAPHQL_PAGE_SIZE}) { nodes { id lastEditedAt author { __typename login avatarUrl } ${REACTION_GROUPS_FIELDS} } } reviewRequests(first: 50) { nodes { requestedReviewer { @@ -942,7 +946,7 @@ export const REVIEW_THREAD_COMMENTS_GRAPHQL_QUERY = `query($owner: String!, $nam pullRequest { id } comments(first: ${GRAPHQL_PAGE_SIZE}, after: $cursor) { pageInfo { hasNextPage endCursor } - nodes { id author { __typename login avatarUrl } body createdAt url ${REACTION_GROUPS_FIELDS} } + nodes { id author { __typename login avatarUrl } body createdAt lastEditedAt url ${REACTION_GROUPS_FIELDS} } } } } @@ -1542,6 +1546,7 @@ function toComments(raw: { author: toActor(comment.author), body: comment.body ?? "", createdAt: comment.createdAt, + editedAt: comment.lastEditedAt ?? null, url: trimmed(comment.url), path: null, reviewState: null, @@ -1566,6 +1571,7 @@ function toComments(raw: { author: toActor(review.author), body: review.body ?? "", createdAt: submittedAt, + editedAt: review.lastEditedAt ?? null, url: trimmed(review.url), path: null, reviewState, @@ -2097,6 +2103,7 @@ export interface GitHubReviewThreadComments { readonly reactions: ReadonlyArray; /** Reactions by node id, for the comments and reviews the `gh` JSON read carries no reaction on. */ readonly reactionsById: ReadonlyMap>; + readonly editedAtById: ReadonlyMap; /** * Everyone on the review: those still asked and those who have already answered. Whoever has * reviewed is no longer an outstanding request, so asking only for requests reports nobody on @@ -2146,6 +2153,7 @@ export interface GitHubReviewThreadPage { * for without any. Only ids with a reaction are here; the rest carry none. */ readonly reactionsById: ReadonlyMap>; + readonly editedAtById: ReadonlyMap; readonly reviewers: ReadonlyArray; readonly avatarsByLogin: ReadonlyMap; readonly botLogins: ReadonlySet; @@ -2176,6 +2184,7 @@ export function reviewThreadConversation( author: comment.author, body: comment.body, createdAt: comment.createdAt, + editedAt: comment.editedAt ?? null, url: comment.url, path: thread.path, reviewState: null, @@ -2275,6 +2284,7 @@ export function decodeReviewThreadsJson( author: toActor(comment.author), body: comment.body ?? "", createdAt: comment.createdAt, + editedAt: comment.lastEditedAt ?? null, url: trimmed(comment.url), reactions: toReactions(comment.reactionGroups, viewer), })), @@ -2341,12 +2351,15 @@ export function decodeReviewThreadsJson( }); } const reactionsById = new Map>(); + const editedAtById = new Map(); for (const node of [ ...(pullRequest.comments?.nodes ?? []), ...(pullRequest.reviews?.nodes ?? []), ]) { const id = trimmed(node.id); if (id === null) continue; + const editedAt = trimmed(node.lastEditedAt); + if (editedAt !== null) editedAtById.set(id, editedAt); const reactions = toReactions(node.reactionGroups, viewer); if (reactions.length > 0) reactionsById.set(id, reactions); } @@ -2355,6 +2368,7 @@ export function decodeReviewThreadsJson( nextCursor: nextCursorOf(threads.pageInfo), reactions: toReactions(pullRequest.reactionGroups, viewer), reactionsById, + editedAtById, reviewers: [...reviewers.values()], avatarsByLogin, botLogins, @@ -2391,6 +2405,7 @@ export function decodeReviewThreadCommentsJson(raw: string): Result.Result< author: toActor(comment.author), body: comment.body ?? "", createdAt: comment.createdAt, + editedAt: comment.lastEditedAt ?? null, url: trimmed(comment.url), reactions: toReactions(comment.reactionGroups, viewer), })), diff --git a/packages/contracts/src/pullRequest.ts b/packages/contracts/src/pullRequest.ts index c8feaad9a41f..ee0a5936c33b 100644 --- a/packages/contracts/src/pullRequest.ts +++ b/packages/contracts/src/pullRequest.ts @@ -203,6 +203,7 @@ export const PullRequestComment = Schema.Struct({ author: Schema.NullOr(PullRequestActor), body: Schema.String, createdAt: IsoDateTime, + editedAt: Schema.optional(Schema.NullOr(IsoDateTime)), url: Schema.NullOr(Schema.String), path: Schema.NullOr(Schema.String), reviewState: Schema.NullOr(Schema.String), @@ -228,6 +229,7 @@ export const PullRequestThreadComment = Schema.Struct({ author: Schema.NullOr(PullRequestActor), body: Schema.String, createdAt: IsoDateTime, + editedAt: Schema.optional(Schema.NullOr(IsoDateTime)), url: Schema.NullOr(Schema.String), reactions: Schema.optional(Schema.Array(PullRequestReaction)), });