mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-04 11:01:16 +01:00
Stop passive comments from waking assignees
This commit is contained in:
1 parent
62c74820cc
commit
4a3174e032
4 files changed
+52
-210
No files matched your search
@@ -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();
|
||||
});
|
||||
});
|
||||
@@ -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 () => {
|
||||
|
||||
@@ -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();
|
||||
});
|
||||
});
|
||||
+33
-123
@@ -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<string, unknown> | 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<string, Parameters<typeof heartbeat.wakeup>[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[] = [];
|
||||
|
||||
Reference in new issue
Block a user