From 2f3d43ea881ca66f274c9398fde2f1b8258312b9 Mon Sep 17 00:00:00 2001 From: Umais-Adeed Date: Wed, 4 Nov 2026 11:40:04 +0500 Subject: [PATCH 1/6] fix(server): a PR watch wakes once for a comment's edits, not on every edit The watch dated a remark by `editedAt ?? createdAt`, so each edit of one bot status comment woke the agent again as "new comments". Hold an edit of a remark the agent already saw until a read finds it unchanged, then wake once and say it was edited. A held edit forces the next sweep to reread remarks. Co-Authored-By: Claude Opus 5.5 --- .../PullRequestWatchReactor.ts | 5 + .../orchestration-v2/pullRequestWatch.test.ts | 59 +++++---- .../src/orchestration-v2/pullRequestWatch.ts | 45 +++++-- .../src/orchestration-v2/runtimeLayer.test.ts | 115 ++++++++++++++++++ packages/contracts/src/threadPullRequest.ts | 4 + 5 files changed, 196 insertions(+), 32 deletions(-) diff --git a/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts b/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts index f37a69c08ee0b..88d1b86eb5cf4 100644 --- a/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts +++ b/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts @@ -152,8 +152,11 @@ function planRead( watch.checksAwaitedSince != null && now - Date.parse(watch.checksAwaitedSince) >= PULL_REQUEST_WATCH_NO_CHECKS_MS, ); + // A held edit is delivered by the next read that finds it unchanged, which a quiet fingerprint skips. + const editsHeld = group.targets.some(({ watch }) => (watch.pendingEdits?.length ?? 0) > 0); if (fingerprint === null) { const read = + editsHeld || last.inFlight || !last.remarksComplete || last.fingerprint !== group.fingerprint || @@ -161,6 +164,7 @@ function planRead( return { detail: read || checksDue, activity: read }; } const activity = + editsHeld || last.remarks !== fingerprint.remarks || !last.remarksComplete || now - last.at >= FINGERPRINT_REREAD_MS; @@ -182,6 +186,7 @@ function watchesEqual(left: ThreadPullRequestWatch, right: ThreadPullRequestWatc (left.approvalAnnounced ?? false) === (right.approvalAnnounced ?? false) && left.remarksThrough === right.remarksThrough && left.remarkIds.join("\n") === right.remarkIds.join("\n") && + JSON.stringify(left.pendingEdits ?? []) === JSON.stringify(right.pendingEdits ?? []) && left.conflicting === right.conflicting && left.wakes === right.wakes && left.checksAwaitedSince === right.checksAwaitedSince diff --git a/apps/server/src/orchestration-v2/pullRequestWatch.test.ts b/apps/server/src/orchestration-v2/pullRequestWatch.test.ts index dece3e3449831..78b942b29343b 100644 --- a/apps/server/src/orchestration-v2/pullRequestWatch.test.ts +++ b/apps/server/src/orchestration-v2/pullRequestWatch.test.ts @@ -402,7 +402,7 @@ describe("evaluatePullRequestWatch", () => { assert.deepEqual(again.next.remarkIds, [first.id, "late"]); }); - it("reports edits after the watermark once and counts them toward the wake limit", () => { + it("reports a settled edit once and counts it toward the wake limit", () => { const old = remark("greptile[bot]", "2026-10-02T11:00:00Z"); const watching = watch({ headSha: "aaaaaaaaaa", @@ -411,19 +411,21 @@ describe("evaluatePullRequestWatch", () => { }); 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, [ + const held = evaluatePullRequestWatch(watching, detail(), [edited]); + assert.deepEqual(held.changes, []); + assert.equal(held.next.remarksThrough, edited.editedAt); + assert.deepEqual(held.next.remarkIds, [old.id]); + const late = { ...remark("reviewer", edited.editedAt), id: "late" }; + const report = evaluatePullRequestWatch(held.next, detail(), [edited, late]); + assert.deepEqual(report.changes, [ { kind: "remarks", remarks: [late] }, + { kind: "remarks", remarks: [edited], edited: true }, ]); + assert.isTrue(report.exhausted); + assert.deepEqual(evaluatePullRequestWatch(report.next, detail(), [edited, late]).changes, []); }); - it("wakes once per edit of a remark seen at the watermark, never for the agent's own", () => { + it("wakes once when a remark's edits settle, never for the agent's own", () => { const seen = remark("ci-bot", "2026-10-10T19:52:40.000Z"); const watching = watch({ headSha: "aaaaaaaaaa", @@ -432,17 +434,32 @@ describe("evaluatePullRequestWatch", () => { wakes: 2, }); assert.deepEqual(evaluatePullRequestWatch(watching, detail(), [seen]).changes, []); - const edited = { ...seen, editedAt: "2026-10-10T20:04:50.000Z" }; - const first = evaluatePullRequestWatch(watching, detail(), [edited]); - assert.deepEqual(first.changes, [{ kind: "remarks", remarks: [edited] }]); - assert.strictEqual(first.next.remarksThrough, edited.editedAt); - assert.deepEqual(evaluatePullRequestWatch(first.next, detail(), [edited]).changes, []); - const again = { ...seen, editedAt: "2026-10-10T20:05:50.000Z" }; - const second = evaluatePullRequestWatch(first.next, detail(), [again]); - assert.deepEqual(second.changes, [{ kind: "remarks", remarks: [again] }]); - assert.deepEqual(evaluatePullRequestWatch(second.next, detail(), [again]).changes, []); - const own = { ...edited, author: { login: "agent-user", name: null, avatarUrl: null } }; - assert.deepEqual(evaluatePullRequestWatch(watching, detail(), [own]).changes, []); + let next = watching; + for (const editedAt of ["2026-10-10T20:04:50.000Z", "2026-10-10T20:06:50.000Z"]) { + const sweep = evaluatePullRequestWatch(next, detail(), [{ ...seen, editedAt }]); + assert.deepEqual(sweep.changes, []); + next = sweep.next; + } + const final = { ...seen, editedAt: "2026-10-10T20:06:50.000Z" }; + // A failed remark read keeps the held edit for the next sweep. + const unread = evaluatePullRequestWatch(next, detail(), null); + assert.deepEqual(unread.changes, []); + const settled = evaluatePullRequestWatch(unread.next, detail(), [final]); + assert.deepEqual(settled.changes, [{ kind: "remarks", remarks: [final], edited: true }]); + assert.deepEqual(evaluatePullRequestWatch(settled.next, detail(), [final]).changes, []); + const own = { ...final, author: { login: "agent-user", name: null, avatarUrl: null } }; + const ownHeld = evaluatePullRequestWatch(watching, detail(), [own]); + assert.deepEqual(evaluatePullRequestWatch(ownHeld.next, detail(), [own]).changes, []); + }); + + it("reports a remark new to the watch at once even when it was already edited", () => { + const posted = { + ...remark("coderabbit[bot]", "2026-10-02T12:06:00Z"), + editedAt: "2026-10-02T12:07:00Z", + }; + assert.deepEqual(evaluatePullRequestWatch(watch(), detail(), [posted]).changes, [ + { kind: "remarks", remarks: [posted] }, + ]); }); it("does not treat a failed check read as a rerun", () => { diff --git a/apps/server/src/orchestration-v2/pullRequestWatch.ts b/apps/server/src/orchestration-v2/pullRequestWatch.ts index 046c48e1858bc..136a9daf96a05 100644 --- a/apps/server/src/orchestration-v2/pullRequestWatch.ts +++ b/apps/server/src/orchestration-v2/pullRequestWatch.ts @@ -18,7 +18,11 @@ export type PullRequestWatchChange = | { readonly kind: "checks-failed"; readonly failed: ReadonlyArray } | { readonly kind: "checks-passed"; readonly count: number; readonly required: boolean } | { readonly kind: "checks-awaiting-approval"; readonly checks: ReadonlyArray } - | { readonly kind: "remarks"; readonly remarks: ReadonlyArray } + | { + readonly kind: "remarks"; + readonly remarks: ReadonlyArray; + readonly edited?: true; + } | { readonly kind: "no-checks"; readonly minutes: number } | { readonly kind: "conflicting" }; @@ -161,14 +165,30 @@ export function evaluatePullRequestWatch( const through = Date.parse(watch.remarksThrough); const activeAt = (remark: PullRequestComment) => remark.editedAt ?? remark.createdAt; // Hosts can stamp remarks to the second, so remarks at the boundary time are told apart by ID. - const fresh = (remarks ?? []).filter((remark) => { - 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 isNew = (at: string, id: string) => + Date.parse(at) > through || (Date.parse(at) === through && !watch.remarkIds.includes(id)); + const byOthers = (remarks ?? []).filter((remark) => remark.author?.login.toLowerCase() !== own); + const fresh = byOthers.filter((remark) => isNew(activeAt(remark), remark.id)); + // Bots rewrite one status comment many times, so an edit of a remark the agent already saw + // waits until a read finds it unchanged and then wakes once with its final text. + const held = new Map((watch.pendingEdits ?? []).map((edit) => [edit.id, edit.editedAt])); + const added: Array = []; + const edits: Array = []; + const heldNow: Array<{ id: string; editedAt: string }> = []; + for (const remark of byOthers) { + if (remark.editedAt != null && held.get(remark.id) === remark.editedAt) { + edits.push(remark); + } else if (!fresh.includes(remark)) { + continue; + } else if (remark.editedAt == null || isNew(remark.createdAt, remark.id)) { + added.push(remark); + } else { + heldNow.push({ id: remark.id, editedAt: remark.editedAt }); + } + } + const pendingEdits = remarks === null ? (watch.pendingEdits ?? []) : heldNow; + if (added.length > 0) changes.push({ kind: "remarks", remarks: added }); + if (edits.length > 0) changes.push({ kind: "remarks", remarks: edits, edited: true }); 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]!); @@ -202,6 +222,7 @@ export function evaluatePullRequestWatch( approvalAnnounced, remarksThrough, remarkIds, + ...(pendingEdits.length > 0 ? { pendingEdits } : {}), conflicting, wakes, checksAwaitedSince, @@ -253,7 +274,7 @@ function changeLines( ]; case "remarks": return [ - `- ${change.remarks.length} new ${change.remarks.length === 1 ? "comment" : "comments"}:`, + `- ${change.remarks.length} ${change.edited ? "edited" : "new"} ${change.remarks.length === 1 ? "comment" : "comments"}:`, ...listed(change.remarks, (remark) => { const where = remark.path === null ? "" : ` on ${remark.path}`; const body = snippet(remark.body); @@ -302,7 +323,9 @@ export function pullRequestWatchMessage(input: { const failed = changes.some( (change) => change.kind === "checks-failed" || change.kind === "conflicting", ); - const summary = changes.map((change) => SUMMARY[change.kind]); + const summary = changes.map((change) => + change.kind === "remarks" && change.edited ? "edited comments" : SUMMARY[change.kind], + ); if (exhausted) summary.push("stopped watching"); return { text, diff --git a/apps/server/src/orchestration-v2/runtimeLayer.test.ts b/apps/server/src/orchestration-v2/runtimeLayer.test.ts index 077980acd2eec..b16125a22f0b6 100644 --- a/apps/server/src/orchestration-v2/runtimeLayer.test.ts +++ b/apps/server/src/orchestration-v2/runtimeLayer.test.ts @@ -4094,6 +4094,121 @@ it.layer(layerTest)("OrchestrationV2LayerLive lifecycle", (it) => { }), ); + it.effect( + "wakes once for a bot comment's edits after they settle under a quiet fingerprint", + () => + Effect.gen(function* () { + const orchestrator = yield* Orchestrator.OrchestratorV2; + const threadId = ThreadId.make("runtime-pull-request-watch-edit-settle"); + const projectId = ProjectId.make("pr-watch-edit-settle-project"); + yield* seedProject({ + projectId, + title: "Watch edits", + workspaceRoot: "/workspace/watch-edits", + defaultModelSelection: null, + createdAt: "2026-10-01T00:00:00.000Z", + }); + yield* orchestrator.dispatch({ + type: "thread.create", + createdBy: "user", + creationSource: "web", + commandId: CommandId.make("pr-watch-edit-settle-create"), + threadId, + projectId, + title: "Watch edits", + modelSelection, + runtimeMode: "full-access", + interactionMode: "default", + branch: null, + worktreePath: null, + }); + const key = { host: "github.com", repository: "pingdotgg/t3code", number: 15 }; + yield* orchestrator.dispatch({ + type: "thread.pull-request.watch", + commandId: CommandId.make("pr-watch-edit-settle-start"), + threadId, + ...key, + watching: true, + link: { url: "https://github.com/pingdotgg/t3code/pull/15", source: "agent" }, + }); + const started = (yield* orchestrator.getThreadShell(threadId))?.pullRequests?.[0]?.watch; + assert.isDefined(started); + if (started === undefined) return; + const at = (minutes: number) => + DateTime.formatIso(DateTime.add(DateTime.makeUnsafe(started.startedAt), { minutes })); + + let editedAt: string | null = null; + const host = Layer.mergeAll( + NodeServices.layer, + NodeCrypto.layer, + Layer.mock(PullRequestService.PullRequestService)({ + watchFingerprint: () => + Effect.sync(() => ({ status: "quiet", remarks: editedAt ?? "none" })), + invalidate: () => Effect.void, + detail: () => + Effect.succeed({ + ...watchedPullRequestDetail({ projectId, number: 15, at: at(-60) }), + checks: [{ name: "lint", status: "success", description: null, url: null }], + }), + activity: () => + Effect.sync(() => ({ + comments: [ + { + id: "review-bot-status", + kind: "issue-comment" as const, + author: { login: "review-bot", name: null, avatarUrl: null }, + body: editedAt === null ? "Reviewing..." : "Review finished.", + createdAt: at(-30), + editedAt, + url: "https://github.com/pingdotgg/t3code/pull/15#status", + path: null, + reviewState: null, + }, + ], + commentCount: 1, + commentsTruncated: false, + reviewThreads: [], + commits: [], + })), + }), + ); + const wakes = Effect.map( + orchestrator.getThreadRecords(threadId, ["messages"]), + ({ messages }) => messages.filter((message) => message.text.includes("review-bot")), + ); + const reactor = yield* PullRequestWatchReactor.make.pipe(Effect.provide(host)); + const worker = yield* EffectWorker.OrchestrationEffectWorkerV2; + const sweep = reactor.sweep.pipe(Effect.andThen(worker.drain())); + + yield* sweep; + for (const minute of [2, 4]) { + editedAt = at(minute); + yield* TestClock.adjust("2 minutes"); + yield* sweep; + } + assert.lengthOf(yield* wakes, 0); + + yield* TestClock.adjust("2 minutes"); + yield* sweep; + const told = yield* wakes; + assert.lengthOf(told, 1); + assert.include(told[0]!.text, '1 edited comment:\n - review-bot: "Review finished."'); + + yield* TestClock.adjust("2 minutes"); + yield* sweep; + assert.lengthOf(yield* wakes, 1); + + yield* orchestrator.dispatch({ + type: "thread.pull-request.watch", + commandId: CommandId.make("pr-watch-edit-settle-end"), + threadId, + ...key, + watching: false, + }); + yield* worker.drain(); + }), + ); + // regression: #21155 watching a pull request again repeated news the thread already got it.effect("watching a dismissed stack layer again keeps what its last watch told", () => Effect.gen(function* () { diff --git a/packages/contracts/src/threadPullRequest.ts b/packages/contracts/src/threadPullRequest.ts index bd0a65a3c0bc9..12ab63fa5d75e 100644 --- a/packages/contracts/src/threadPullRequest.ts +++ b/packages/contracts/src/threadPullRequest.ts @@ -96,6 +96,10 @@ export const ThreadPullRequestWatch = Schema.Struct({ remarksThrough: IsoDateTime, /** Reported remarks at remarksThrough, so late arrivals at that timestamp still count. */ remarkIds: Schema.Array(TrimmedNonEmptyString), + /** Edits of remarks the agent already saw, held until a read finds them unchanged. */ + pendingEdits: Schema.optional( + Schema.Array(Schema.Struct({ id: TrimmedNonEmptyString, editedAt: IsoDateTime })), + ), conflicting: Schema.Boolean, /** Consecutive comment-only wakes; limited to prevent bot loops. */ wakes: NonNegativeInt, From 5e373a007f6878c4678554b0652cac44e33b4ccd Mon Sep 17 00:00:00 2001 From: Umais-Adeed Date: Wed, 4 Nov 2026 11:52:34 +0500 Subject: [PATCH 2/6] fix(server): confirm a held PR comment edit from fresh review-thread pages Cached review-thread tails survive an edit, because neither the comment count nor the top-level watermark changes. The confirming read could then report a stale held edit as settled. Drop cached tails while a watch holds an edit. Co-Authored-By: Claude Opus 5.5 --- .../PullRequestWatchReactor.ts | 10 +- .../src/orchestration-v2/runtimeLayer.test.ts | 130 ++++++++++++++++++ 2 files changed, 137 insertions(+), 3 deletions(-) diff --git a/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts b/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts index 88d1b86eb5cf4..af88f3db275ef 100644 --- a/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts +++ b/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts @@ -137,6 +137,9 @@ const snapshotFingerprint = ({ link }: WatchTarget) => { ].join(" "); }; +const holdsEdits = (group: WatchGroup) => + group.targets.some(({ watch }) => (watch.pendingEdits?.length ?? 0) > 0); + function planRead( group: WatchGroup, last: LastRead | undefined, @@ -153,7 +156,7 @@ function planRead( now - Date.parse(watch.checksAwaitedSince) >= PULL_REQUEST_WATCH_NO_CHECKS_MS, ); // A held edit is delivered by the next read that finds it unchanged, which a quiet fingerprint skips. - const editsHeld = group.targets.some(({ watch }) => (watch.pendingEdits?.length ?? 0) > 0); + const editsHeld = holdsEdits(group); if (fingerprint === null) { const read = editsHeld || @@ -476,9 +479,10 @@ export const make = Effect.gen(function* () { if ( last === undefined || group.targets.some((target) => !last.watches.has(watchKey(target))) || - (fingerprint !== null && last.remarks !== fingerprint.remarks) + (fingerprint !== null && last.remarks !== fingerprint.remarks) || + holdsEdits(group) ) { - // A new watch or changed host remarks needs a fresh view of every tail. + // A new watch, changed host remarks, or a held edit needs a fresh view of every tail. threadTails.delete(group.key); } const reference = referenceOf(group); diff --git a/apps/server/src/orchestration-v2/runtimeLayer.test.ts b/apps/server/src/orchestration-v2/runtimeLayer.test.ts index b16125a22f0b6..217ef2e8a6b37 100644 --- a/apps/server/src/orchestration-v2/runtimeLayer.test.ts +++ b/apps/server/src/orchestration-v2/runtimeLayer.test.ts @@ -4209,6 +4209,136 @@ it.layer(layerTest)("OrchestrationV2LayerLive lifecycle", (it) => { }), ); + it.effect("confirms a held edit in a paginated review thread from a fresh page", () => + Effect.gen(function* () { + const orchestrator = yield* Orchestrator.OrchestratorV2; + const threadId = ThreadId.make("runtime-pull-request-watch-edit-tail"); + const projectId = ProjectId.make("pr-watch-edit-tail-project"); + yield* seedProject({ + projectId, + title: "Watch tail edits", + workspaceRoot: "/workspace/watch-tail-edits", + defaultModelSelection: null, + createdAt: "2026-10-01T00:00:00.000Z", + }); + yield* orchestrator.dispatch({ + type: "thread.create", + createdBy: "user", + creationSource: "web", + commandId: CommandId.make("pr-watch-edit-tail-create"), + threadId, + projectId, + title: "Watch tail edits", + modelSelection, + runtimeMode: "full-access", + interactionMode: "default", + branch: null, + worktreePath: null, + }); + const key = { host: "github.com", repository: "pingdotgg/t3code", number: 16 }; + yield* orchestrator.dispatch({ + type: "thread.pull-request.watch", + commandId: CommandId.make("pr-watch-edit-tail-start"), + threadId, + ...key, + watching: true, + link: { url: "https://github.com/pingdotgg/t3code/pull/16", source: "agent" }, + }); + const started = (yield* orchestrator.getThreadShell(threadId))?.pullRequests?.[0]?.watch; + assert.isDefined(started); + if (started === undefined) return; + const at = (minutes: number) => + DateTime.formatIso(DateTime.add(DateTime.makeUnsafe(started.startedAt), { minutes })); + const comment = (id: string, body: string, editedAt: string | null) => ({ + id, + author: { login: "review-bot", name: null, avatarUrl: null }, + body, + createdAt: at(-30), + editedAt, + url: null, + }); + + let tail: { body: string; editedAt: string | null } = { + body: "Reviewing...", + editedAt: null, + }; + const host = Layer.mergeAll( + NodeServices.layer, + NodeCrypto.layer, + Layer.mock(PullRequestService.PullRequestService)({ + // Fingerprints miss review-thread comment edits, so the host reports one throughout. + watchFingerprint: () => Effect.succeed({ status: "quiet", remarks: "same" }), + invalidate: () => Effect.void, + detail: () => + Effect.succeed({ + ...watchedPullRequestDetail({ projectId, number: 16, at: at(-60) }), + checks: [{ name: "lint", status: "success", description: null, url: null }], + }), + activity: () => + Effect.succeed({ + comments: [], + commentCount: 0, + commentsTruncated: false, + reviewThreadsTruncated: false, + reviewThreads: [ + { + id: "bot-thread", + path: "src/index.ts", + line: 1, + side: "right", + isResolved: false, + isOutdated: false, + comments: [comment("bot-first", "Started.", null)], + commentCount: 2, + nextCommentsCursor: "after-1", + }, + ], + commits: [], + }), + threadComments: () => + Effect.sync(() => ({ + comments: [comment("bot-status", tail.body, tail.editedAt)], + nextCursor: null, + })), + }), + ); + const wakes = Effect.map( + orchestrator.getThreadRecords(threadId, ["messages"]), + ({ messages }) => messages.filter((message) => message.text.includes("review-bot")), + ); + const reactor = yield* PullRequestWatchReactor.make.pipe(Effect.provide(host)); + const worker = yield* EffectWorker.OrchestrationEffectWorkerV2; + const sweep = reactor.sweep.pipe(Effect.andThen(worker.drain())); + + yield* sweep; + tail = { body: "Still reviewing...", editedAt: at(31) }; + yield* TestClock.adjust("31 minutes"); + yield* sweep; + tail = { body: "Review finished.", editedAt: at(33) }; + yield* TestClock.adjust("2 minutes"); + yield* sweep; + assert.lengthOf(yield* wakes, 0); + + yield* TestClock.adjust("2 minutes"); + yield* sweep; + const told = yield* wakes; + assert.lengthOf(told, 1); + assert.include( + told[0]!.text, + '1 edited comment:\n - review-bot on src/index.ts: "Review finished."', + ); + + yield* orchestrator.dispatch({ + type: "thread.pull-request.watch", + commandId: CommandId.make("pr-watch-edit-tail-end"), + threadId, + ...key, + watching: false, + }); + yield* worker.drain(); + }), + ); + // regression: #21155 watching a pull request again repeated news the thread already got it.effect("watching a dismissed stack layer again keeps what its last watch told", () => Effect.gen(function* () { From 6f317ce05babb04b36c945d9f1d30bcc2081675a Mon Sep 17 00:00:00 2001 From: maria-rcks Date: Wed, 4 Nov 2026 09:02:25 +0000 Subject: [PATCH 3/6] fix(server): preserve GitCafe comment edit timestamps --- .../src/server/gitCafePullRequestJson.test.ts | 14 +++++++++++++- .../src/server/gitCafePullRequestJson.ts | 5 +++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/packages/source-control-gitcafe/src/server/gitCafePullRequestJson.test.ts b/packages/source-control-gitcafe/src/server/gitCafePullRequestJson.test.ts index 83279771cb4f0..818f4a9cbd05e 100644 --- a/packages/source-control-gitcafe/src/server/gitCafePullRequestJson.test.ts +++ b/packages/source-control-gitcafe/src/server/gitCafePullRequestJson.test.ts @@ -188,18 +188,20 @@ describe("toActivity", () => { assert.strictEqual(result.reviewThreads[0]?.comments.length, 2); }); - it("carries per-remark edit permission onto both copies of a remark", () => { + it("carries per-remark edit metadata onto both copies of a remark", () => { const result = activity([ { ...lineComment, id: "root", threadId: "root", + updatedAt: timestamp, capabilities: capabilities({ edit: false, resolve: true }), }, { ...lineComment, id: "reply", threadId: "root", + updatedAt: "2026-09-12T12:02:00Z", capabilities: capabilities({ edit: true }), }, { ...lineComment, id: "silent", threadId: "silent", path: null, line: null, side: null }, @@ -208,6 +210,16 @@ describe("toActivity", () => { assert.strictEqual(canEdit("root"), false); assert.strictEqual(canEdit("reply"), true); assert.isUndefined(canEdit("silent")); + assert.isUndefined(result.comments.find((comment) => comment.id === "root")?.editedAt); + assert.isUndefined(result.comments.find((comment) => comment.id === "silent")?.editedAt); + assert.strictEqual( + result.comments.find((comment) => comment.id === "reply")?.editedAt, + "2026-09-12T12:02:00Z", + ); + assert.strictEqual( + result.reviewThreads[0]?.comments.find((comment) => comment.id === "reply")?.editedAt, + "2026-09-12T12:02:00Z", + ); assert.deepStrictEqual( result.reviewThreads[0]?.comments.map((comment) => [comment.id, comment.canEdit]), [ diff --git a/packages/source-control-gitcafe/src/server/gitCafePullRequestJson.ts b/packages/source-control-gitcafe/src/server/gitCafePullRequestJson.ts index cc692637b82fe..0322eaaf808e2 100644 --- a/packages/source-control-gitcafe/src/server/gitCafePullRequestJson.ts +++ b/packages/source-control-gitcafe/src/server/gitCafePullRequestJson.ts @@ -117,6 +117,7 @@ const GitCafeComment = Schema.Struct({ author: Actor, body: Schema.NullOr(Schema.String), createdAt: IsoDateTime, + updatedAt: Schema.optional(IsoDateTime), path: Schema.NullOr(Schema.String), // Only positive lines anchor review threads; the API allows any integer. line: Schema.NullOr(Schema.Int), @@ -482,6 +483,10 @@ export function toActivity(input: { author: toActor(comment.author, host), body: comment.body ?? "", createdAt: comment.createdAt, + ...(comment.updatedAt !== undefined && + Date.parse(comment.updatedAt) > Date.parse(comment.createdAt) + ? { editedAt: comment.updatedAt } + : {}), url: null, ...(comment.capabilities === undefined ? {} : { canEdit: comment.capabilities.edit }), ...(reactions === undefined ? {} : { reactions: reactions.byComment.get(comment.id) ?? [] }), From e0c638a9ce895c68709b036ad3c4862d8c68b71b Mon Sep 17 00:00:00 2001 From: Umais-Adeed Date: Wed, 4 Nov 2026 14:28:35 +0500 Subject: [PATCH 4/6] fix(server): keep a held PR comment edit across a rewatch and fresh reads Watching a pull request again now carries the held edit forward, so the settled text still reaches the agent. A watch holding an edit also drops cached activity before reading, even without a host fingerprint. Co-Authored-By: Claude Opus 5.5 --- apps/server/src/orchestration-v2/Orchestrator.ts | 3 +++ .../src/orchestration-v2/PullRequestWatchReactor.ts | 2 +- apps/server/src/orchestration-v2/runtimeLayer.test.ts | 10 ++++++++++ 3 files changed, 14 insertions(+), 1 deletion(-) diff --git a/apps/server/src/orchestration-v2/Orchestrator.ts b/apps/server/src/orchestration-v2/Orchestrator.ts index 9f22aa28c3ba2..f3399acc022b0 100644 --- a/apps/server/src/orchestration-v2/Orchestrator.ts +++ b/apps/server/src/orchestration-v2/Orchestrator.ts @@ -4167,6 +4167,9 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio ...(existing.endedWatch?.checksAwaitedSince !== undefined && { checksAwaitedSince: existing.endedWatch.checksAwaitedSince, }), + ...(existing.endedWatch?.pendingEdits !== undefined && { + pendingEdits: existing.endedWatch.pendingEdits, + }), remarksThrough: startedAt, remarkIds: [], wakes: 0, diff --git a/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts b/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts index af88f3db275ef..b10ffccacf6c3 100644 --- a/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts +++ b/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts @@ -487,7 +487,7 @@ export const make = Effect.gen(function* () { } const reference = referenceOf(group); // Invalidate cached reads before using the newer fingerprint; clients receive no notification. - if (fingerprint !== null) yield* pullRequests.invalidate({ reference }); + if (fingerprint !== null || holdsEdits(group)) yield* pullRequests.invalidate({ reference }); const [detailRead, activityRead] = yield* Effect.all( [ Effect.exit(pullRequests.detail({ ...reference, allowStale: false })), diff --git a/apps/server/src/orchestration-v2/runtimeLayer.test.ts b/apps/server/src/orchestration-v2/runtimeLayer.test.ts index 217ef2e8a6b37..c7c31364e995a 100644 --- a/apps/server/src/orchestration-v2/runtimeLayer.test.ts +++ b/apps/server/src/orchestration-v2/runtimeLayer.test.ts @@ -4188,6 +4188,16 @@ it.layer(layerTest)("OrchestrationV2LayerLive lifecycle", (it) => { } assert.lengthOf(yield* wakes, 0); + // Watching again keeps the edit the last watch was holding. + for (const watching of [false, true]) { + yield* orchestrator.dispatch({ + type: "thread.pull-request.watch", + commandId: CommandId.make(`pr-watch-edit-settle-rewatch-${watching}`), + threadId, + ...key, + watching, + }); + } yield* TestClock.adjust("2 minutes"); yield* sweep; const told = yield* wakes; From 96f93bc037ad5d9b818411eb6db151dee52ad843 Mon Sep 17 00:00:00 2001 From: Umais-Adeed Date: Tue, 10 Nov 2026 10:19:33 +0500 Subject: [PATCH 5/6] fix(server): keep a held PR comment edit the bot changed while the watch was stopped Watching again starts remarksThrough at the restart, so a held comment edited while the watch was stopped looked old and its hold was dropped. A held comment with a new edit is now held again at that edit and delivered once it settles. Co-Authored-By: Claude Opus 5.5 --- apps/server/src/orchestration-v2/pullRequestWatch.test.ts | 8 ++++++++ apps/server/src/orchestration-v2/pullRequestWatch.ts | 5 ++++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/apps/server/src/orchestration-v2/pullRequestWatch.test.ts b/apps/server/src/orchestration-v2/pullRequestWatch.test.ts index 78b942b29343b..ea24d8a0fcf83 100644 --- a/apps/server/src/orchestration-v2/pullRequestWatch.test.ts +++ b/apps/server/src/orchestration-v2/pullRequestWatch.test.ts @@ -447,6 +447,14 @@ describe("evaluatePullRequestWatch", () => { const settled = evaluatePullRequestWatch(unread.next, detail(), [final]); assert.deepEqual(settled.changes, [{ kind: "remarks", remarks: [final], edited: true }]); assert.deepEqual(evaluatePullRequestWatch(settled.next, detail(), [final]).changes, []); + // A watch started again keeps the held edit, even one the bot changed while it was stopped. + const rewatched = { ...next, remarksThrough: "2026-10-10T20:10:00.000Z", remarkIds: [] }; + const whileStopped = { ...seen, editedAt: "2026-10-10T20:08:50.000Z" }; + const reheld = evaluatePullRequestWatch(rewatched, detail(), [whileStopped]); + assert.deepEqual(reheld.changes, []); + assert.deepEqual(evaluatePullRequestWatch(reheld.next, detail(), [whileStopped]).changes, [ + { kind: "remarks", remarks: [whileStopped], edited: true }, + ]); const own = { ...final, author: { login: "agent-user", name: null, avatarUrl: null } }; const ownHeld = evaluatePullRequestWatch(watching, detail(), [own]); assert.deepEqual(evaluatePullRequestWatch(ownHeld.next, detail(), [own]).changes, []); diff --git a/apps/server/src/orchestration-v2/pullRequestWatch.ts b/apps/server/src/orchestration-v2/pullRequestWatch.ts index 136a9daf96a05..04fbfda94fa4f 100644 --- a/apps/server/src/orchestration-v2/pullRequestWatch.ts +++ b/apps/server/src/orchestration-v2/pullRequestWatch.ts @@ -179,7 +179,10 @@ export function evaluatePullRequestWatch( if (remark.editedAt != null && held.get(remark.id) === remark.editedAt) { edits.push(remark); } else if (!fresh.includes(remark)) { - continue; + // A rewatch moves the watermark past an edit made while the watch was stopped. + if (remark.editedAt != null && held.has(remark.id)) { + heldNow.push({ id: remark.id, editedAt: remark.editedAt }); + } } else if (remark.editedAt == null || isNew(remark.createdAt, remark.id)) { added.push(remark); } else { From afac023e1edf412787e8a1e6f30662211cdd1ee2 Mon Sep 17 00:00:00 2001 From: Royalaid <2439803+royalaid@users.noreply.github.com> Date: Fri, 13 Nov 2026 11:01:37 -0800 Subject: [PATCH 6/6] fix(server): a PR watch reports a comment's edit once per head A bot that keeps rewriting one comment on the same head woke the agent once per rewrite. CodeRabbit, while a review is rate limited, rewrites its walkthrough with a countdown every few minutes; each rewrite stays unchanged for longer than a sweep, so each was held, found settled and delivered. CI status comments rewritten per job do the same. The watch now records the remarks whose edit it reported on the current head and neither holds nor reports their later edits until the head moves. The first edit after a push still wakes, so a bot that re-reviews a push by editing its summary is reported (#15282). A remark new to the watch, and other remarks' first edits, are unaffected. Watching again keeps the record, like the other news the last watch told; watches saved before the field load unchanged. --- .../src/orchestration-v2/Orchestrator.ts | 3 ++ .../PullRequestWatchReactor.ts | 1 + .../orchestration-v2/pullRequestWatch.test.ts | 36 +++++++++++++++++++ .../src/orchestration-v2/pullRequestWatch.ts | 7 +++- packages/contracts/src/threadPullRequest.ts | 2 ++ 5 files changed, 48 insertions(+), 1 deletion(-) diff --git a/apps/server/src/orchestration-v2/Orchestrator.ts b/apps/server/src/orchestration-v2/Orchestrator.ts index 79abd3527dbf7..927d168b61b22 100644 --- a/apps/server/src/orchestration-v2/Orchestrator.ts +++ b/apps/server/src/orchestration-v2/Orchestrator.ts @@ -4290,6 +4290,9 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio ...(existing.endedWatch?.pendingEdits !== undefined && { pendingEdits: existing.endedWatch.pendingEdits, }), + ...(existing.endedWatch?.remarkEditsTold !== undefined && { + remarkEditsTold: existing.endedWatch.remarkEditsTold, + }), remarksThrough: startedAt, remarkIds: [], wakes: 0, diff --git a/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts b/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts index b10ffccacf6c3..9beb387578180 100644 --- a/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts +++ b/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts @@ -190,6 +190,7 @@ function watchesEqual(left: ThreadPullRequestWatch, right: ThreadPullRequestWatc left.remarksThrough === right.remarksThrough && left.remarkIds.join("\n") === right.remarkIds.join("\n") && JSON.stringify(left.pendingEdits ?? []) === JSON.stringify(right.pendingEdits ?? []) && + (left.remarkEditsTold ?? []).join("\n") === (right.remarkEditsTold ?? []).join("\n") && left.conflicting === right.conflicting && left.wakes === right.wakes && left.checksAwaitedSince === right.checksAwaitedSince diff --git a/apps/server/src/orchestration-v2/pullRequestWatch.test.ts b/apps/server/src/orchestration-v2/pullRequestWatch.test.ts index ea24d8a0fcf83..d403eb6fd3612 100644 --- a/apps/server/src/orchestration-v2/pullRequestWatch.test.ts +++ b/apps/server/src/orchestration-v2/pullRequestWatch.test.ts @@ -460,6 +460,42 @@ describe("evaluatePullRequestWatch", () => { assert.deepEqual(evaluatePullRequestWatch(ownHeld.next, detail(), [own]).changes, []); }); + it("reports a remark's settled edit once per head, and again after a push", () => { + const summary = remark("coderabbitai[bot]", "2026-10-10T19:52:40.000Z"); + const status = remark("ci-bot", "2026-10-10T19:52:40.000Z"); + let next = watch({ + headSha: "aaaaaaaaaa", + remarksThrough: summary.createdAt, + remarkIds: [summary.id, status.id], + }); + // One read sees the edit and holds it; the next finds it unchanged and reports it. + const sweep = (headSha: string, edited: ReadonlyArray) => { + const held = evaluatePullRequestWatch(next, detail({ headSha }), edited); + const settled = evaluatePullRequestWatch(held.next, detail({ headSha }), edited); + next = settled.next; + return [...held.changes, ...settled.changes].filter((change) => change.kind === "remarks"); + }; + const at = (minute: string) => ({ ...summary, editedAt: `2026-10-10T20:${minute}:00.000Z` }); + // A rate-limited bot rewrites its countdown every few minutes on the same head. + assert.deepEqual(sweep("aaaaaaaaaa", [at("00")]), [ + { kind: "remarks", remarks: [at("00")], edited: true }, + ]); + for (const minute of ["05", "10", "15"]) { + assert.deepEqual(sweep("aaaaaaaaaa", [at(minute)]), []); + } + assert.isUndefined(next.pendingEdits); + // Another remark's first edit on this head is still news. + const statusEdit = { ...status, editedAt: "2026-10-10T20:16:00.000Z" }; + assert.deepEqual(sweep("aaaaaaaaaa", [at("15"), statusEdit]), [ + { kind: "remarks", remarks: [statusEdit], edited: true }, + ]); + // A bot re-reviewing a push by editing its summary is reported again. + assert.deepEqual(sweep("bbbbbbbbbb", [at("20"), statusEdit]), [ + { kind: "remarks", remarks: [at("20")], edited: true }, + ]); + assert.deepEqual(sweep("bbbbbbbbbb", [at("25"), statusEdit]), []); + }); + it("reports a remark new to the watch at once even when it was already edited", () => { const posted = { ...remark("coderabbit[bot]", "2026-10-02T12:06:00Z"), diff --git a/apps/server/src/orchestration-v2/pullRequestWatch.ts b/apps/server/src/orchestration-v2/pullRequestWatch.ts index 04fbfda94fa4f..d713b4eb08e68 100644 --- a/apps/server/src/orchestration-v2/pullRequestWatch.ts +++ b/apps/server/src/orchestration-v2/pullRequestWatch.ts @@ -172,6 +172,9 @@ export function evaluatePullRequestWatch( // Bots rewrite one status comment many times, so an edit of a remark the agent already saw // waits until a read finds it unchanged and then wakes once with its final text. const held = new Map((watch.pendingEdits ?? []).map((edit) => [edit.id, edit.editedAt])); + // Some bots keep rewriting one comment on the same head (a review countdown, a CI status + // table), so each remark's edit is reported once per head; after a push its edits are news again. + const editsTold = headMoved ? [] : (watch.remarkEditsTold ?? []); const added: Array = []; const edits: Array = []; const heldNow: Array<{ id: string; editedAt: string }> = []; @@ -185,11 +188,12 @@ export function evaluatePullRequestWatch( } } else if (remark.editedAt == null || isNew(remark.createdAt, remark.id)) { added.push(remark); - } else { + } else if (!editsTold.includes(remark.id)) { heldNow.push({ id: remark.id, editedAt: remark.editedAt }); } } const pendingEdits = remarks === null ? (watch.pendingEdits ?? []) : heldNow; + const remarkEditsTold = [...new Set([...editsTold, ...edits.map((remark) => remark.id)])]; if (added.length > 0) changes.push({ kind: "remarks", remarks: added }); if (edits.length > 0) changes.push({ kind: "remarks", remarks: edits, edited: true }); const latest = Math.max(through, ...fresh.map((remark) => Date.parse(activeAt(remark)))); @@ -226,6 +230,7 @@ export function evaluatePullRequestWatch( remarksThrough, remarkIds, ...(pendingEdits.length > 0 ? { pendingEdits } : {}), + ...(remarkEditsTold.length > 0 ? { remarkEditsTold } : {}), conflicting, wakes, checksAwaitedSince, diff --git a/packages/contracts/src/threadPullRequest.ts b/packages/contracts/src/threadPullRequest.ts index 12ab63fa5d75e..a7b67f05c8f30 100644 --- a/packages/contracts/src/threadPullRequest.ts +++ b/packages/contracts/src/threadPullRequest.ts @@ -100,6 +100,8 @@ export const ThreadPullRequestWatch = Schema.Struct({ pendingEdits: Schema.optional( Schema.Array(Schema.Struct({ id: TrimmedNonEmptyString, editedAt: IsoDateTime })), ), + /** Remarks whose edit was reported on headSha; their later edits wait for a new head. */ + remarkEditsTold: Schema.optional(Schema.Array(TrimmedNonEmptyString)), conflicting: Schema.Boolean, /** Consecutive comment-only wakes; limited to prevent bot loops. */ wakes: NonNegativeInt,