authorgravatar for 75276669+Gigioxx@users.noreply.github.comGuillermo Casanova <75276669+Gigioxx@users.noreply.github.com> 2026-10-05 18:20:38-03:00
committergravatar for noreply@github.comGitHub <noreply@github.com> 2026-10-05 14:20:38-07:00
log21eb9a421aff7a8fbe044b1a34bb3e364fb94a68
tree61de166464a3f95c63ea4785b264cb51cc021ef5
parentf973b1d7d5cd7ffb4dd3c1c61cc0caece09fd1a6
signature Signed by PGP key B5690EEEBB952194

fix(server): PR watch wakes the agent when a bot edits its review comment (#15415)

8 files changed, 89 insertions(+), 10 deletions(-)

apps/server/src/orchestration-v2/pullRequestWatch.test.ts+21-1
......@@ -1,7 +1,6 @@
11import type {
22 PullRequestCheck,
33 PullRequestComment,
4 PullRequestDetail,
54 ThreadPullRequestWatch,
65} from "@t3tools/contracts";
76import { assert, describe, it } from "@effect/vitest";
......@@ -119,6 +118,27 @@ describe("evaluatePullRequestWatch", () => {
119118 assert.deepEqual(again.next.remarkIds, [first.id, "late"]);
120119 });
121120
121 it("reports edits after the watermark once and counts them toward the wake limit", () => {
122 const old = remark("greptile[bot]", "2026-10-02T11:00:00Z");
123 const watching = watch({
124 headSha: "aaaaaaaaaa",
125 remarkIds: [old.id],
126 wakes: PULL_REQUEST_WATCH_WAKE_LIMIT - 1,
127 });
128 assert.deepEqual(evaluatePullRequestWatch(watching, detail(), [old]).changes, []);
129 const edited = { ...old, editedAt: "2026-10-02T12:06:00Z" };
130 const report = evaluatePullRequestWatch(watching, detail(), [edited]);
131 assert.deepEqual(report.changes, [{ kind: "remarks", remarks: [edited] }]);
132 assert.equal(report.next.remarksThrough, edited.editedAt);
133 assert.deepEqual(report.next.remarkIds, [old.id]);
134 assert.isTrue(report.exhausted);
135 assert.deepEqual(evaluatePullRequestWatch(report.next, detail(), [edited]).changes, []);
136 const late = { ...edited, id: "late" };
137 assert.deepEqual(evaluatePullRequestWatch(report.next, detail(), [edited, late]).changes, [
138 { kind: "remarks", remarks: [late] },
139 ]);
140 });
141
122142 it("does not treat a failed check read as a rerun", () => {
123143 const failed = detail({ checks: [check("lint", "failure")] });
124144 const reported = evaluatePullRequestWatch(watch(), failed, noRemarks);
apps/server/src/orchestration-v2/pullRequestWatch.ts+6-4
......@@ -71,18 +71,20 @@ export function evaluatePullRequestWatch(
7171
7272 const own = (detail.viewer ?? detail.author?.login)?.toLowerCase();
7373 const through = Date.parse(watch.remarksThrough);
74 // An edit counts as new activity, so bots that rewrite one summary comment still wake the agent.
75 const activeAt = (remark: PullRequestComment) => remark.editedAt ?? remark.createdAt;
7476 // GitHub times are per second, so remarks at the boundary time are told apart by ID.
7577 const fresh = (remarks ?? []).filter((remark) => {
76 const at = Date.parse(remark.createdAt);
78 const at = Date.parse(activeAt(remark));
7779 return (
7880 (at > through || (at === through && !watch.remarkIds.includes(remark.id))) &&
7981 remark.author?.login.toLowerCase() !== own
8082 );
8183 });
8284 if (fresh.length > 0) changes.push({ kind: "remarks", remarks: fresh });
83 const latest = Math.max(through, ...fresh.map((remark) => Date.parse(remark.createdAt)));
84 const atLatest = fresh.filter((remark) => Date.parse(remark.createdAt) === latest);
85 const remarksThrough = latest === through ? watch.remarksThrough : atLatest[0]!.createdAt;
85 const latest = Math.max(through, ...fresh.map((remark) => Date.parse(activeAt(remark))));
86 const atLatest = fresh.filter((remark) => Date.parse(activeAt(remark)) === latest);
87 const remarksThrough = latest === through ? watch.remarksThrough : activeAt(atLatest[0]!);
8688 const remarkIds = [
8789 ...(latest === through ? watch.remarkIds : []),
8890 ...atLatest.map((remark) => remark.id),
apps/server/src/pullRequest/GitHubPullRequestCli.ts+3
......@@ -2338,6 +2338,7 @@ export const make = Effect.gen(function* () {
23382338 let reviewers: ReadonlyArray<PullRequestActor> = [];
23392339 let reactions: GitHubReviewThreadPage["reactions"] = [];
23402340 const reactionsById = new Map<string, ReadonlyArray<PullRequestReaction>>();
2341 const editedAtById = new Map<string, string>();
23412342 let commits: GitHubReviewThreadPage["commits"] = [];
23422343 let viewer: GitHubReviewThreadPage["viewer"] = { canUpdate: true, didAuthor: false };
23432344 const dismissalsByReviewId = new Map<string, string>();
......@@ -2356,6 +2357,7 @@ export const make = Effect.gen(function* () {
23562357 reviewers = read.reviewers;
23572358 reactions = read.reactions;
23582359 for (const [id, entry] of read.reactionsById) reactionsById.set(id, entry);
2360 for (const [id, editedAt] of read.editedAtById) editedAtById.set(id, editedAt);
23592361 commits = read.commits;
23602362 viewer = read.viewer;
23612363 for (const [id, message] of read.dismissalsByReviewId)
......@@ -2412,6 +2414,7 @@ export const make = Effect.gen(function* () {
24122414 reviewThreadsTruncated: cursor !== null,
24132415 reactions,
24142416 reactionsById,
2417 editedAtById,
24152418 reviewers,
24162419 avatarsByLogin,
24172420 botLogins,
apps/server/src/pullRequest/GitHubPullRequestProvider.test.ts+3
......@@ -885,6 +885,7 @@ describe("getChangeRequest commits", () => {
885885 reviewThreadsTruncated: false,
886886 reactions: [],
887887 reactionsById: new Map<string, ReadonlyArray<PullRequestReaction>>(),
888 editedAtById: new Map<string, string>(),
888889 reviewers: [],
889890 avatarsByLogin: new Map<string, string>(),
890891 botLogins: new Set<string>(),
......@@ -971,6 +972,7 @@ describe("getChangeRequestActivity dismissed reviews", () => {
971972 reviewThreadsTruncated: false,
972973 reactions: [],
973974 reactionsById: new Map(),
975 editedAtById: new Map([["PRR_1", "2026-07-04T00:00:00Z"]]),
974976 reviewers: [],
975977 avatarsByLogin: new Map(),
976978 botLogins: new Set(["macroscopeapp"]),
......@@ -1002,6 +1004,7 @@ describe("getChangeRequestActivity dismissed reviews", () => {
10021004 Effect.map((activity) => {
10031005 expect(activity.comments[0]?.body).toBe("Dismissing prior approval to re-evaluate 9b66581");
10041006 expect(activity.comments[0]?.author?.isBot).toBe(true);
1007 expect(activity.comments[0]?.editedAt).toBe("2026-07-04T00:00:00Z");
10051008 }),
10061009 Effect.provide(layerFor("<!-- Macroscope (Approvability) review body marker -->")),
10071010 ),
apps/server/src/pullRequest/GitHubPullRequestProvider.ts+2
......@@ -409,6 +409,7 @@ export const make = Effect.gen(function* () {
409409 dismissalsByReviewId: new Map<string, string>(),
410410 reactions: [],
411411 reactionsById: new Map<string, ReadonlyArray<PullRequestReaction>>(),
412 editedAtById: new Map<string, string>(),
412413 reviewThreads: [],
413414 commentCount: 0,
414415 truncated: true,
......@@ -475,6 +476,7 @@ export const make = Effect.gen(function* () {
475476 // A comment out of `gh pr view --json` carries none of its own: that read
476477 // reports no reaction at all, so they arrive from the GraphQL page by node id.
477478 reactions: comment.reactions ?? reviewThreads.reactionsById.get(comment.id) ?? [],
479 editedAt: reviewThreads.editedAtById.get(comment.id) ?? comment.editedAt ?? null,
478480 }))
479481 .toSorted((left, right) => left.createdAt.localeCompare(right.createdAt)),
480482 // `gh pr view --json comments,reviews` follows GitHub's cursors itself, so those two
apps/server/src/pullRequest/gitHubPullRequestJson.test.ts+33-1
......@@ -1105,7 +1105,13 @@ describe("review thread decoding", () => {
11051105 path: "src/a.ts",
11061106 line: 42,
11071107 diffSide: "LEFT",
1108 comments: { totalCount: 2, nodes: [comment("c1", "first"), comment("c2", "second")] },
1108 comments: {
1109 totalCount: 2,
1110 nodes: [
1111 { ...comment("c1", "first"), lastEditedAt: "2026-07-02T00:00:00Z" },
1112 comment("c2", "second"),
1113 ],
1114 },
11091115 },
11101116 ]),
11111117 ),
......@@ -1124,6 +1130,7 @@ describe("review thread decoding", () => {
11241130 author: { login: "bilal", name: null, avatarUrl: "https://avatars/b.png" },
11251131 body: "first",
11261132 createdAt: "2026-07-01T00:00:00Z",
1133 editedAt: "2026-07-02T00:00:00Z",
11271134 url: "https://github.com/acme/web/pull/1#discussion_rc1",
11281135 reactions: [],
11291136 },
......@@ -1132,12 +1139,16 @@ describe("review thread decoding", () => {
11321139 author: { login: "bilal", name: null, avatarUrl: "https://avatars/b.png" },
11331140 body: "second",
11341141 createdAt: "2026-07-01T00:00:00Z",
1142 editedAt: null,
11351143 url: "https://github.com/acme/web/pull/1#discussion_rc2",
11361144 reactions: [],
11371145 },
11381146 ],
11391147 },
11401148 ]);
1149 expect(
1150 reviewThreadConversation(reviewThreads.threads.map((entry) => entry.thread))[0]?.editedAt,
1151 ).toBe("2026-07-02T00:00:00Z");
11411152 });
11421153
11431154 it("leaves an outdated thread without a line rather than pinning it to a stale one", () => {
......@@ -1186,6 +1197,27 @@ describe("review thread decoding", () => {
11861197 expect(threads).toHaveLength(1);
11871198 });
11881199
1200 it("reads edit times for issue comments and reviews without using reaction updates", () => {
1201 const editedAt = "2026-10-02T12:06:00Z";
1202 const result = expectSuccess(
1203 decodeReviewThreadsJson(
1204 threadsJson([], {
1205 comments: {
1206 nodes: [
1207 { id: "edited", lastEditedAt: editedAt },
1208 { id: "reaction-only", lastEditedAt: null, updatedAt: editedAt },
1209 ],
1210 },
1211 reviews: { nodes: [{ id: "review", lastEditedAt: editedAt }] },
1212 }),
1213 ),
1214 );
1215 expect([...result.editedAtById]).toEqual([
1216 ["edited", editedAt],
1217 ["review", editedAt],
1218 ]);
1219 });
1220
11891221 it("puts an issue comment's and a review's reactions in reactionsById, and the pull request's own in reactions", () => {
11901222 const result = expectSuccess(
11911223 decodeReviewThreadsJson(
apps/server/src/pullRequest/gitHubPullRequestJson.ts+19-4
......@@ -362,6 +362,7 @@ function toReactions(
362362
363363const RawCommentSchema = Schema.Struct({
364364 id: Schema.String,
365 lastEditedAt: Schema.optional(Schema.NullOr(Schema.String)),
365366 author: Schema.optional(Schema.NullOr(RawActorSchema)),
366367 body: Schema.optional(Schema.String),
367368 createdAt: Schema.String,
......@@ -372,6 +373,7 @@ const RawCommentSchema = Schema.Struct({
372373
373374const RawReviewSchema = Schema.Struct({
374375 id: Schema.String,
376 lastEditedAt: Schema.optional(Schema.NullOr(Schema.String)),
375377 author: Schema.optional(Schema.NullOr(RawActorSchema)),
376378 body: Schema.optional(Schema.String),
377379 state: Schema.optional(Schema.NullOr(Schema.String)),
......@@ -490,6 +492,7 @@ const RawReviewThreadsSchema = Schema.Struct({
490492 Schema.Struct({
491493 id: Schema.optional(Schema.NullOr(Schema.String)),
492494 author: Schema.optional(Schema.NullOr(RawActorSchema)),
495 lastEditedAt: Schema.optional(Schema.NullOr(Schema.String)),
493496 reactionGroups: RawReactionGroupsSchema,
494497 }),
495498 ),
......@@ -507,6 +510,7 @@ const RawReviewThreadsSchema = Schema.Struct({
507510 Schema.Struct({
508511 id: Schema.optional(Schema.NullOr(Schema.String)),
509512 author: Schema.optional(Schema.NullOr(RawActorSchema)),
513 lastEditedAt: Schema.optional(Schema.NullOr(Schema.String)),
510514 reactionGroups: RawReactionGroupsSchema,
511515 }),
512516 ),
......@@ -885,7 +889,7 @@ export const REVIEW_THREADS_GRAPHQL_QUERY = `query($owner: String!, $name: Strin
885889 comments(first: 10) {
886890 totalCount
887891 pageInfo { hasNextPage endCursor }
888 nodes { id author { __typename login avatarUrl } body createdAt url ${REACTION_GROUPS_FIELDS} }
892 nodes { id author { __typename login avatarUrl } body createdAt lastEditedAt url ${REACTION_GROUPS_FIELDS} }
889893 }
890894 }
891895 }
......@@ -894,9 +898,9 @@ export const REVIEW_THREADS_GRAPHQL_QUERY = `query($owner: String!, $name: Strin
894898 author { __typename login avatarUrl }
895899 ${REACTION_GROUPS_FIELDS}
896900 comments(first: ${GRAPHQL_PAGE_SIZE}) {
897 nodes { id author { __typename login avatarUrl } ${REACTION_GROUPS_FIELDS} }
901 nodes { id lastEditedAt author { __typename login avatarUrl } ${REACTION_GROUPS_FIELDS} }
898902 }
899 reviews(first: ${GRAPHQL_PAGE_SIZE}) { nodes { id author { __typename login avatarUrl } ${REACTION_GROUPS_FIELDS} } }
903 reviews(first: ${GRAPHQL_PAGE_SIZE}) { nodes { id lastEditedAt author { __typename login avatarUrl } ${REACTION_GROUPS_FIELDS} } }
900904 reviewRequests(first: 50) {
901905 nodes {
902906 requestedReviewer {
......@@ -942,7 +946,7 @@ export const REVIEW_THREAD_COMMENTS_GRAPHQL_QUERY = `query($owner: String!, $nam
942946 pullRequest { id }
943947 comments(first: ${GRAPHQL_PAGE_SIZE}, after: $cursor) {
944948 pageInfo { hasNextPage endCursor }
945 nodes { id author { __typename login avatarUrl } body createdAt url ${REACTION_GROUPS_FIELDS} }
949 nodes { id author { __typename login avatarUrl } body createdAt lastEditedAt url ${REACTION_GROUPS_FIELDS} }
946950 }
947951 }
948952 }
......@@ -1542,6 +1546,7 @@ function toComments(raw: {
15421546 author: toActor(comment.author),
15431547 body: comment.body ?? "",
15441548 createdAt: comment.createdAt,
1549 editedAt: comment.lastEditedAt ?? null,
15451550 url: trimmed(comment.url),
15461551 path: null,
15471552 reviewState: null,
......@@ -1566,6 +1571,7 @@ function toComments(raw: {
15661571 author: toActor(review.author),
15671572 body: review.body ?? "",
15681573 createdAt: submittedAt,
1574 editedAt: review.lastEditedAt ?? null,
15691575 url: trimmed(review.url),
15701576 path: null,
15711577 reviewState,
......@@ -2098,6 +2104,7 @@ export interface GitHubReviewThreadComments {
20982104 readonly reactions: ReadonlyArray<PullRequestReaction>;
20992105 /** Reactions by node id, for the comments and reviews the `gh` JSON read carries no reaction on. */
21002106 readonly reactionsById: ReadonlyMap<string, ReadonlyArray<PullRequestReaction>>;
2107 readonly editedAtById: ReadonlyMap<string, string>;
21012108 /**
21022109 * Everyone on the review: those still asked and those who have already answered. Whoever has
21032110 * reviewed is no longer an outstanding request, so asking only for requests reports nobody on
......@@ -2147,6 +2154,7 @@ export interface GitHubReviewThreadPage {
21472154 * for without any. Only ids with a reaction are here; the rest carry none.
21482155 */
21492156 readonly reactionsById: ReadonlyMap<string, ReadonlyArray<PullRequestReaction>>;
2157 readonly editedAtById: ReadonlyMap<string, string>;
21502158 readonly reviewers: ReadonlyArray<PullRequestActor>;
21512159 readonly avatarsByLogin: ReadonlyMap<string, string>;
21522160 readonly botLogins: ReadonlySet<string>;
......@@ -2177,6 +2185,7 @@ export function reviewThreadConversation(
21772185 author: comment.author,
21782186 body: comment.body,
21792187 createdAt: comment.createdAt,
2188 editedAt: comment.editedAt ?? null,
21802189 url: comment.url,
21812190 path: thread.path,
21822191 reviewState: null,
......@@ -2276,6 +2285,7 @@ export function decodeReviewThreadsJson(
22762285 author: toActor(comment.author),
22772286 body: comment.body ?? "",
22782287 createdAt: comment.createdAt,
2288 editedAt: comment.lastEditedAt ?? null,
22792289 url: trimmed(comment.url),
22802290 reactions: toReactions(comment.reactionGroups, viewer),
22812291 })),
......@@ -2342,12 +2352,15 @@ export function decodeReviewThreadsJson(
23422352 });
23432353 }
23442354 const reactionsById = new Map<string, ReadonlyArray<PullRequestReaction>>();
2355 const editedAtById = new Map<string, string>();
23452356 for (const node of [
23462357 ...(pullRequest.comments?.nodes ?? []),
23472358 ...(pullRequest.reviews?.nodes ?? []),
23482359 ]) {
23492360 const id = trimmed(node.id);
23502361 if (id === null) continue;
2362 const editedAt = trimmed(node.lastEditedAt);
2363 if (editedAt !== null) editedAtById.set(id, editedAt);
23512364 const reactions = toReactions(node.reactionGroups, viewer);
23522365 if (reactions.length > 0) reactionsById.set(id, reactions);
23532366 }
......@@ -2356,6 +2369,7 @@ export function decodeReviewThreadsJson(
23562369 nextCursor: nextCursorOf(threads.pageInfo),
23572370 reactions: toReactions(pullRequest.reactionGroups, viewer),
23582371 reactionsById,
2372 editedAtById,
23592373 reviewers: [...reviewers.values()],
23602374 avatarsByLogin,
23612375 botLogins,
......@@ -2392,6 +2406,7 @@ export function decodeReviewThreadCommentsJson(raw: string): Result.Result<
23922406 author: toActor(comment.author),
23932407 body: comment.body ?? "",
23942408 createdAt: comment.createdAt,
2409 editedAt: comment.lastEditedAt ?? null,
23952410 url: trimmed(comment.url),
23962411 reactions: toReactions(comment.reactionGroups, viewer),
23972412 })),
packages/contracts/src/pullRequest.ts+2
......@@ -203,6 +203,7 @@ export const PullRequestComment = Schema.Struct({
203203 author: Schema.NullOr(PullRequestActor),
204204 body: Schema.String,
205205 createdAt: IsoDateTime,
206 editedAt: Schema.optional(Schema.NullOr(IsoDateTime)),
206207 url: Schema.NullOr(Schema.String),
207208 path: Schema.NullOr(Schema.String),
208209 reviewState: Schema.NullOr(Schema.String),
......@@ -228,6 +229,7 @@ export const PullRequestThreadComment = Schema.Struct({
228229 author: Schema.NullOr(PullRequestActor),
229230 body: Schema.String,
230231 createdAt: IsoDateTime,
232 editedAt: Schema.optional(Schema.NullOr(IsoDateTime)),
231233 url: Schema.NullOr(Schema.String),
232234 reactions: Schema.optional(Schema.Array(PullRequestReaction)),
233235});