From 4a3174e03214cd04fcabc2ee092fa375a56b11d7 Mon Sep 17 00:00:00 2001 From: Dotta Date: Sat, 6 Jun 2026 23:19:00 +0000 Subject: [PATCH] Stop passive comments from waking assignees --- .../document-annotation-routes.test.ts | 13 +- .../issue-comment-reopen-routes.test.ts | 65 ++------ ...issue-update-comment-wakeup-routes.test.ts | 28 +--- server/src/routes/issues.ts | 156 ++++-------------- 4 files changed, 52 insertions(+), 210 deletions(-) diff --git a/server/src/__tests__/document-annotation-routes.test.ts b/server/src/__tests__/document-annotation-routes.test.ts index 0a2aa0e6784..f7f4d1997ac 100644 --- a/server/src/__tests__/document-annotation-routes.test.ts +++ b/server/src/__tests__/document-annotation-routes.test.ts @@ -237,7 +237,7 @@ describe("document annotation routes", () => { }); }); - it("creates annotation threads, syncs references, logs activity, and wakes the assignee", async () => { + it("creates annotation threads, syncs references, logs activity, and does not wake the assignee", async () => { mockIssueService.getById.mockResolvedValue({ id: issueId, companyId, @@ -261,15 +261,7 @@ describe("document annotation routes", () => { expect(mockLogActivity).toHaveBeenCalledWith(expect.anything(), expect.objectContaining({ action: "issue.document_annotation_thread_created", })); - expect(mockHeartbeatService.wakeup).toHaveBeenCalledWith( - "99999999-9999-4999-8999-999999999999", - expect.objectContaining({ - payload: expect.objectContaining({ - annotationThreadId: annotationThread.id, - annotationCommentId: annotationComment.id, - }), - }), - ); + expect(mockHeartbeatService.wakeup).not.toHaveBeenCalled(); }); it("rejects agent cross-company annotation reads", async () => { @@ -293,5 +285,6 @@ describe("document annotation routes", () => { expect(mockLogActivity).toHaveBeenCalledWith(expect.anything(), expect.objectContaining({ action: "issue.document_annotation_thread_resolved", })); + expect(mockHeartbeatService.wakeup).not.toHaveBeenCalled(); }); }); diff --git a/server/src/__tests__/issue-comment-reopen-routes.test.ts b/server/src/__tests__/issue-comment-reopen-routes.test.ts index 4043d86a37b..d56164cadca 100644 --- a/server/src/__tests__/issue-comment-reopen-routes.test.ts +++ b/server/src/__tests__/issue-comment-reopen-routes.test.ts @@ -639,20 +639,8 @@ describe.sequential("issue comment reopen routes", () => { }), }), ); - await waitForWakeup(() => expect(mockHeartbeatService.wakeup).toHaveBeenCalledWith( - "22222222-2222-4222-8222-222222222222", - expect.objectContaining({ - reason: "issue_commented", - payload: expect.objectContaining({ - commentId: "comment-1", - mutation: "comment", - }), - contextSnapshot: expect.objectContaining({ - wakeReason: "issue_commented", - source: "issue.comment", - }), - }), - )); + await waitForWakeup(() => expect(mockIssueService.findMentionedAgents).toHaveBeenCalled()); + expect(mockHeartbeatService.wakeup).not.toHaveBeenCalled(); }); it("does not move scheduled-retry issues to todo when POST comment retry cancellation fails", async () => { @@ -701,12 +689,8 @@ describe.sequential("issue comment reopen routes", () => { expect(mockIssueService.getCurrentScheduledRetry).toHaveBeenCalledWith("11111111-1111-4111-8111-111111111111"); expect(mockIssueService.update).not.toHaveBeenCalled(); expect(mockHeartbeatService.cancelRun).not.toHaveBeenCalled(); - await waitForWakeup(() => expect(mockHeartbeatService.wakeup).toHaveBeenCalledWith( - "22222222-2222-4222-8222-222222222222", - expect.objectContaining({ - reason: "issue_commented", - }), - )); + await waitForWakeup(() => expect(mockIssueService.findMentionedAgents).toHaveBeenCalled()); + expect(mockHeartbeatService.wakeup).not.toHaveBeenCalled(); }); it("passes validated comment presentation fields to trusted board comment writes", async () => { @@ -808,21 +792,8 @@ describe.sequential("issue comment reopen routes", () => { expect(res.status).toBe(201); expect(mockIssueService.update).not.toHaveBeenCalled(); - await waitForWakeup(() => expect(mockHeartbeatService.wakeup).toHaveBeenCalledWith( - "22222222-2222-4222-8222-222222222222", - expect.objectContaining({ - reason: "issue_commented", - payload: expect.objectContaining({ - commentId: "comment-1", - mutation: "comment", - }), - contextSnapshot: expect.objectContaining({ - issueId: "11111111-1111-4111-8111-111111111111", - wakeCommentId: "comment-1", - wakeReason: "issue_commented", - }), - }), - )); + await waitForWakeup(() => expect(mockIssueService.findMentionedAgents).toHaveBeenCalled()); + expect(mockHeartbeatService.wakeup).not.toHaveBeenCalled(); }); it("does not implicitly reopen closed issues via POST comments when no agent is assigned", async () => { @@ -917,16 +888,8 @@ describe.sequential("issue comment reopen routes", () => { }), ); expect(mockHeartbeatService.cancelRun).toHaveBeenCalledWith("retry-run-1"); - await waitForWakeup(() => expect(mockHeartbeatService.wakeup).toHaveBeenCalledWith( - "22222222-2222-4222-8222-222222222222", - expect.objectContaining({ - reason: "issue_commented", - payload: expect.objectContaining({ - commentId: "comment-1", - mutation: "comment", - }), - }), - )); + await waitForWakeup(() => expect(mockIssueService.findMentionedAgents).toHaveBeenCalled()); + expect(mockHeartbeatService.wakeup).not.toHaveBeenCalled(); }); it("does not move scheduled-retry issues to todo when PATCH comment retry cancellation fails", async () => { @@ -1028,16 +991,8 @@ describe.sequential("issue comment reopen routes", () => { "11111111-1111-4111-8111-111111111111", expect.objectContaining({ status: "todo" }), ); - await waitForWakeup(() => expect(mockHeartbeatService.wakeup).toHaveBeenCalledWith( - "22222222-2222-4222-8222-222222222222", - expect.objectContaining({ - reason: "issue_commented", - payload: expect.objectContaining({ - commentId: "comment-1", - mutation: "comment", - }), - }), - )); + await waitForWakeup(() => expect(mockIssueService.findMentionedAgents).toHaveBeenCalled()); + expect(mockHeartbeatService.wakeup).not.toHaveBeenCalled(); }); it("wakes the assignee when an assigned blocked issue moves back to todo", async () => { diff --git a/server/src/__tests__/issue-update-comment-wakeup-routes.test.ts b/server/src/__tests__/issue-update-comment-wakeup-routes.test.ts index 89b1b257de2..dceb8e4e26e 100644 --- a/server/src/__tests__/issue-update-comment-wakeup-routes.test.ts +++ b/server/src/__tests__/issue-update-comment-wakeup-routes.test.ts @@ -269,7 +269,7 @@ describe("issue update comment wakeups", () => { ); }); - it("wakes the assignee on comment-only issue updates", async () => { + it("does not wake the assignee on comment-only issue updates", async () => { const existing = makeIssue({ assigneeAgentId: ASSIGNEE_AGENT_ID, assigneeUserId: null, @@ -292,26 +292,10 @@ describe("issue update comment wakeups", () => { }); expect(res.status).toBe(200); - expect(mockHeartbeatService.wakeup).toHaveBeenCalledTimes(1); - expect(mockHeartbeatService.wakeup).toHaveBeenCalledWith( - ASSIGNEE_AGENT_ID, - expect.objectContaining({ - source: "automation", - reason: "issue_commented", - payload: expect.objectContaining({ - issueId: existing.id, - commentId: "comment-2", - mutation: "comment", - }), - contextSnapshot: expect.objectContaining({ - issueId: existing.id, - taskId: existing.id, - commentId: "comment-2", - wakeCommentId: "comment-2", - wakeReason: "issue_commented", - source: "issue.comment", - }), - }), - ); + await vi.waitFor(() => expect(mockIssueService.findMentionedAgents).toHaveBeenCalledWith( + existing.companyId, + "please revise this", + )); + expect(mockHeartbeatService.wakeup).not.toHaveBeenCalled(); }); }); diff --git a/server/src/routes/issues.ts b/server/src/routes/issues.ts index 40768a74124..47d84ed35a4 100644 --- a/server/src/routes/issues.ts +++ b/server/src/routes/issues.ts @@ -1431,46 +1431,6 @@ export function issueRoutes( }; } - function queueAnnotationCommentWakeup(input: { - issue: { id: string; assigneeAgentId: string | null; status: string }; - actor: { actorType: "user" | "agent"; actorId: string }; - threadId: string; - commentId: string; - documentKey: string; - }) { - const assigneeId = input.issue.assigneeAgentId; - const selfComment = input.actor.actorType === "agent" && input.actor.actorId === assigneeId; - if (!assigneeId || selfComment || isClosedIssueStatus(input.issue.status)) return; - void heartbeat.wakeup(assigneeId, { - source: "automation", - triggerDetail: "system", - reason: "issue_commented", - payload: { - issueId: input.issue.id, - annotationThreadId: input.threadId, - annotationCommentId: input.commentId, - documentKey: input.documentKey, - mutation: "document_annotation_comment", - }, - requestedByActorType: input.actor.actorType, - requestedByActorId: input.actor.actorId, - contextSnapshot: { - issueId: input.issue.id, - taskId: input.issue.id, - annotationThreadId: input.threadId, - annotationCommentId: input.commentId, - documentKey: input.documentKey, - source: "issue.document.annotation", - wakeReason: "issue_commented", - }, - }).catch((err) => logger.warn({ - err, - issueId: input.issue.id, - annotationThreadId: input.threadId, - annotationCommentId: input.commentId, - }, "failed to wake assignee on document annotation comment")); - } - async function canonicalizePaperclipArtifactMetadata(input: { issue: { id: string; companyId: string }; metadata: Record | null | undefined; @@ -3135,16 +3095,6 @@ export function issueRoutes( }, }); - if (firstComment) { - queueAnnotationCommentWakeup({ - issue, - actor, - threadId: thread.id, - commentId: firstComment.id, - documentKey: thread.documentKey, - }); - } - res.status(201).json(thread); }, ); @@ -3227,14 +3177,6 @@ export function issueRoutes( }, }); - queueAnnotationCommentWakeup({ - issue, - actor, - threadId: comment.threadId, - commentId: comment.id, - documentKey: keyParsed.data, - }); - res.status(201).json(comment); }, ); @@ -5546,20 +5488,17 @@ export function issueRoutes( if (commentBody && comment) { const assigneeId = issue.assigneeAgentId; - const actorIsAgent = actor.actorType === "agent"; - const selfComment = actorIsAgent && actor.actorId === assigneeId; - const skipAssigneeCommentWake = selfComment || isClosed; - if (assigneeId && !assigneeChanged && (reopened || !skipAssigneeCommentWake)) { + if (assigneeId && !assigneeChanged && reopened) { addWakeup(assigneeId, { source: "automation", triggerDetail: "system", - reason: reopened ? "issue_reopened_via_comment" : "issue_commented", + reason: "issue_reopened_via_comment", payload: { issueId: id, commentId: comment.id, mutation: "comment", - ...(reopened ? { reopenedFrom: reopenFromStatus } : {}), + reopenedFrom: reopenFromStatus, ...(resumeRequested === true ? { resumeIntent: true, followUpRequested: true } : {}), ...(interruptedRunId ? { interruptedRunId } : {}), }, @@ -5570,9 +5509,9 @@ export function issueRoutes( taskId: id, commentId: comment.id, wakeCommentId: comment.id, - source: reopened ? "issue.comment.reopen" : "issue.comment", - wakeReason: reopened ? "issue_reopened_via_comment" : "issue_commented", - ...(reopened ? { reopenedFrom: reopenFromStatus } : {}), + source: "issue.comment.reopen", + wakeReason: "issue_reopened_via_comment", + reopenedFrom: reopenFromStatus, ...(resumeRequested === true ? { resumeIntent: true, followUpRequested: true } : {}), ...(interruptedRunId ? { interruptedRunId } : {}), }, @@ -6714,62 +6653,33 @@ export function issueRoutes( const wakeups = new Map[1]>(); const assigneeId = currentIssue.assigneeAgentId; const actorIsAgent = actor.actorType === "agent"; - const selfComment = actorIsAgent && actor.actorId === assigneeId; - const skipWake = selfComment || isClosed; - if (assigneeId && (reopened || !skipWake)) { - if (reopened) { - wakeups.set(assigneeId, { - source: "automation", - triggerDetail: "system", - reason: "issue_reopened_via_comment", - payload: { - issueId: currentIssue.id, - commentId: comment.id, - reopenedFrom: reopenFromStatus, - mutation: "comment", - ...(resumeRequested === true ? { resumeIntent: true, followUpRequested: true } : {}), - ...(interruptedRunId ? { interruptedRunId } : {}), - }, - requestedByActorType: actor.actorType, - requestedByActorId: actor.actorId, - contextSnapshot: { - issueId: currentIssue.id, - taskId: currentIssue.id, - commentId: comment.id, - wakeCommentId: comment.id, - source: "issue.comment.reopen", - wakeReason: "issue_reopened_via_comment", - reopenedFrom: reopenFromStatus, - ...(resumeRequested === true ? { resumeIntent: true, followUpRequested: true } : {}), - ...(interruptedRunId ? { interruptedRunId } : {}), - }, - }); - } else { - wakeups.set(assigneeId, { - source: "automation", - triggerDetail: "system", - reason: "issue_commented", - payload: { - issueId: currentIssue.id, - commentId: comment.id, - mutation: "comment", - ...(resumeRequested === true ? { resumeIntent: true, followUpRequested: true } : {}), - ...(interruptedRunId ? { interruptedRunId } : {}), - }, - requestedByActorType: actor.actorType, - requestedByActorId: actor.actorId, - contextSnapshot: { - issueId: currentIssue.id, - taskId: currentIssue.id, - commentId: comment.id, - wakeCommentId: comment.id, - source: "issue.comment", - wakeReason: "issue_commented", - ...(resumeRequested === true ? { resumeIntent: true, followUpRequested: true } : {}), - ...(interruptedRunId ? { interruptedRunId } : {}), - }, - }); - } + if (assigneeId && reopened) { + wakeups.set(assigneeId, { + source: "automation", + triggerDetail: "system", + reason: "issue_reopened_via_comment", + payload: { + issueId: currentIssue.id, + commentId: comment.id, + reopenedFrom: reopenFromStatus, + mutation: "comment", + ...(resumeRequested === true ? { resumeIntent: true, followUpRequested: true } : {}), + ...(interruptedRunId ? { interruptedRunId } : {}), + }, + requestedByActorType: actor.actorType, + requestedByActorId: actor.actorId, + contextSnapshot: { + issueId: currentIssue.id, + taskId: currentIssue.id, + commentId: comment.id, + wakeCommentId: comment.id, + source: "issue.comment.reopen", + wakeReason: "issue_reopened_via_comment", + reopenedFrom: reopenFromStatus, + ...(resumeRequested === true ? { resumeIntent: true, followUpRequested: true } : {}), + ...(interruptedRunId ? { interruptedRunId } : {}), + }, + }); } let mentionedIds: string[] = [];