diff --git a/apps/server/src/orchestration-v2/Orchestrator.ts b/apps/server/src/orchestration-v2/Orchestrator.ts index 3157f55cacfa6..927d168b61b22 100644 --- a/apps/server/src/orchestration-v2/Orchestrator.ts +++ b/apps/server/src/orchestration-v2/Orchestrator.ts @@ -4287,6 +4287,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 f37a69c08ee0b..9beb387578180 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, @@ -152,8 +155,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 || @@ -161,6 +167,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 +189,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.wakes === right.wakes && left.checksAwaitedSince === right.checksAwaitedSince @@ -471,14 +480,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 dece3e3449831..d403eb6fd3612 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,76 @@ 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 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"), + 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..d713b4eb08e68 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,37 @@ 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])); + // 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 }> = []; + for (const remark of byOthers) { + if (remark.editedAt != null && held.get(remark.id) === remark.editedAt) { + 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 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)))); const atLatest = fresh.filter((remark) => Date.parse(activeAt(remark)) === latest); const remarksThrough = latest === through ? watch.remarksThrough : activeAt(atLatest[0]!); @@ -202,6 +229,8 @@ export function evaluatePullRequestWatch( approvalAnnounced, remarksThrough, remarkIds, + ...(pendingEdits.length > 0 ? { pendingEdits } : {}), + ...(remarkEditsTold.length > 0 ? { remarkEditsTold } : {}), conflicting, wakes, checksAwaitedSince, @@ -253,7 +282,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 +331,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 a49e323770cc1..41a77a9bbef2e 100644 --- a/apps/server/src/orchestration-v2/runtimeLayer.test.ts +++ b/apps/server/src/orchestration-v2/runtimeLayer.test.ts @@ -4112,6 +4112,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 bd0a65a3c0bc9..a7b67f05c8f30 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 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, 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) ?? [] }),