diff --git a/apps/server/src/orchestration-v2/Orchestrator.ts b/apps/server/src/orchestration-v2/Orchestrator.ts index 00f9dfae8d9e1..a64ed34929159 100644 --- a/apps/server/src/orchestration-v2/Orchestrator.ts +++ b/apps/server/src/orchestration-v2/Orchestrator.ts @@ -4913,6 +4913,12 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio ...(existing.endedWatch?.checksAwaitedSince !== undefined && { checksAwaitedSince: existing.endedWatch.checksAwaitedSince, }), + ...(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 ddfe74160459a..9f2cfd9fabc8f 100644 --- a/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts +++ b/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts @@ -156,6 +156,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, @@ -173,8 +176,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 = holdsEdits(group); if (fingerprint === null) { const read = + editsHeld || last.inFlight || !last.remarksComplete || last.fingerprint !== group.fingerprint || @@ -182,6 +188,7 @@ function planRead( return { detail: read || checksDue, activity: read }; } const activity = + editsHeld || last.remarks !== fingerprint.remarks || !last.remarksComplete || now - last.at >= FINGERPRINT_REREAD_MS; @@ -207,6 +214,8 @@ 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.remarkEditsTold ?? []).join("\n") === (right.remarkEditsTold ?? []).join("\n") && left.conflicting === right.conflicting && left.mergeQueueRemovalId === right.mergeQueueRemovalId && left.wakes === right.wakes && @@ -590,14 +599,15 @@ 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); // 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/pullRequestWatch.test.ts b/apps/server/src/orchestration-v2/pullRequestWatch.test.ts index eb2ae2756d540..a445de0de3926 100644 --- a/apps/server/src/orchestration-v2/pullRequestWatch.test.ts +++ b/apps/server/src/orchestration-v2/pullRequestWatch.test.ts @@ -600,7 +600,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", @@ -609,19 +609,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", @@ -630,17 +632,95 @@ 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, []); + // 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, []); + }); + + it("reports a remark's later edits on one head only after they settle for longer", () => { + 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], + }); + const time = (minute: number) => `2026-10-10T20:${String(minute).padStart(2, "0")}:00.000Z`; + // Reads at each minute, returning the remark changes they report. + const reads = ( + headSha: string, + seen: ReadonlyArray, + minutes: ReadonlyArray, + ) => + minutes.flatMap((minute) => { + const report = evaluate(next, detail({ headSha }), seen, Date.parse(time(minute))); + next = report.next; + return report.changes.filter((change) => change.kind === "remarks"); + }); + const at = (minute: number) => ({ ...summary, editedAt: time(minute) }); + // A remark's first edit on a head wakes once it reads unchanged. + assert.deepEqual(reads("aaaaaaaaaa", [at(0)], [1, 3]), [ + { kind: "remarks", remarks: [at(0)], edited: true }, + ]); + // A rate-limited bot rewriting its countdown stays quiet until the rewrites stop. + for (const minute of [5, 10, 15]) { + assert.deepEqual(reads("aaaaaaaaaa", [at(minute)], [minute + 1, minute + 3]), []); + } + assert.deepEqual(reads("aaaaaaaaaa", [at(15)], [26]), [ + { kind: "remarks", remarks: [at(15)], edited: true }, + ]); + // Another remark's first edit on this head is still news. + const statusEdit = { ...status, editedAt: time(27) }; + assert.deepEqual(reads("aaaaaaaaaa", [at(15), statusEdit], [28, 30]), [ + { kind: "remarks", remarks: [statusEdit], edited: true }, + ]); + // After a push the bot edits its summary to reviewing, then to its finished review. + assert.deepEqual(reads("bbbbbbbbbb", [at(31), statusEdit], [32, 34]), [ + { kind: "remarks", remarks: [at(31)], edited: true }, + ]); + assert.deepEqual(reads("bbbbbbbbbb", [at(35), statusEdit], [36, 38]), []); + assert.deepEqual(reads("bbbbbbbbbb", [at(35), statusEdit], [46]), [ + { kind: "remarks", remarks: [at(35)], edited: true }, + ]); + // An edit held on the old head does not spend the new head's report. + const stale = { ...status, editedAt: time(47) }; + assert.deepEqual(reads("bbbbbbbbbb", [at(35), stale], [48]), []); + assert.deepEqual(reads("cccccccccc", [at(35), stale], [50, 52]), []); + const finding = { ...status, editedAt: time(53) }; + assert.deepEqual(reads("cccccccccc", [at(35), finding], [54, 56]), [ + { kind: "remarks", remarks: [finding], edited: true }, + ]); + }); + + 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 e12d9a124df8a..d63955c0c5b13 100644 --- a/apps/server/src/orchestration-v2/pullRequestWatch.ts +++ b/apps/server/src/orchestration-v2/pullRequestWatch.ts @@ -11,6 +11,8 @@ import * as DateTime from "effect/DateTime"; export const PULL_REQUEST_WATCH_WAKE_LIMIT = 10; /** How long a head may go without any check before the agent is told none reported. */ export const PULL_REQUEST_WATCH_NO_CHECKS_MS = 15 * 60_000; +/** Outlasts a bot's countdown rewrites, so a remark already reported on a head wakes once more. */ +const TOLD_EDIT_SETTLE_MS = 10 * 60_000; const LISTED_ITEMS = 10; const SNIPPET_LENGTH = 200; @@ -18,7 +20,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" } | { readonly kind: "merge-queue-removed"; readonly reason: string | null }; @@ -180,14 +186,42 @@ 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. + // An edit held on an older head describes that head, so a push drops it. + const pending = headMoved ? [] : (watch.pendingEdits ?? []); + const held = new Map(pending.map((edit) => [edit.id, edit.editedAt])); + // Some bots keep rewriting one comment on the same head (a review countdown, a CI status + // table), so after a remark's edit is reported on a head its later edits settle for longer. + const editsTold = headMoved ? [] : (watch.remarkEditsTold ?? []); + 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) { + const settling = + editsTold.includes(remark.id) && now - Date.parse(remark.editedAt) < TOLD_EDIT_SETTLE_MS; + if (settling) heldNow.push({ id: remark.id, editedAt: remark.editedAt }); + else edits.push(remark); + } else if (!fresh.includes(remark)) { + // 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 { + heldNow.push({ id: remark.id, editedAt: remark.editedAt }); + } + } + const pendingEdits = remarks === null ? pending : 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)))); const atLatest = fresh.filter((remark) => Date.parse(activeAt(remark)) === latest); const remarksThrough = latest === through ? watch.remarksThrough : activeAt(atLatest[0]!); @@ -238,6 +272,8 @@ export function evaluatePullRequestWatch( approvalAnnounced, remarksThrough, remarkIds, + ...(pendingEdits.length > 0 ? { pendingEdits } : {}), + ...(remarkEditsTold.length > 0 ? { remarkEditsTold } : {}), conflicting, mergeQueueRemovalId, wakes, @@ -296,7 +332,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); @@ -390,7 +426,9 @@ export function pullRequestWatchMessage(input: { change.kind === "conflicting" || change.kind === "merge-queue-removed", ); - 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 c7cc43ee5ddea..12995aaf63295 100644 --- a/apps/server/src/orchestration-v2/runtimeLayer.test.ts +++ b/apps/server/src/orchestration-v2/runtimeLayer.test.ts @@ -5116,6 +5116,261 @@ 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); + + // 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; + 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(); + }), + ); + + 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* () { diff --git a/packages/contracts/src/threadPullRequest.ts b/packages/contracts/src/threadPullRequest.ts index 7775f0755ff0b..7b7d326632eec 100644 --- a/packages/contracts/src/threadPullRequest.ts +++ b/packages/contracts/src/threadPullRequest.ts @@ -96,6 +96,12 @@ 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 })), + ), + /** Remarks whose edit was reported on headSha; their later edits settle for longer. */ + remarkEditsTold: Schema.optional(Schema.Array(TrimmedNonEmptyString)), conflicting: Schema.Boolean, /** Last removal seen; null after an empty read, absent before the first read. */ mergeQueueRemovalId: Schema.optional(Schema.NullOr(TrimmedNonEmptyString)), diff --git a/packages/source-control-gitcafe/src/server/gitCafePullRequestJson.test.ts b/packages/source-control-gitcafe/src/server/gitCafePullRequestJson.test.ts index 46170704c5a1b..b04a8e9c40cca 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) ?? [] }),