[codex] Guard document comment wake boundaries (#7766)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - The execution control plane uses issue comments, assignments, monitors, blockers, and interactions to decide when agent-owned work should wake and run. > - Top-level issue comments are actionable issue-thread feedback for the assignee, but document-scoped comments are review context unless they are converted into an explicit routing primitive. > - Document annotation comments were still wired into the same `issue_commented` wake path as top-level issue comments. > - That made document activity capable of waking an assignee and looking like an execution path even when no issue-level handoff happened. > - This pull request narrows the wake boundary so document annotation activity stays document-scoped while normal issue comments continue waking the assignee. > - The benefit is fewer spurious wakeups and clearer non-terminal issue liveness semantics. ## Linked Issues or Issue Description Internal Paperclip work: [PAP-10613](/PAP/issues/PAP-10613), [PAP-10640](/PAP/issues/PAP-10640) Problem description: - Document annotation thread creation and annotation comments were treated as assignee wake sources. - Document-scoped activity should remain visible as document/review context, but should not by itself act as a queued issue wake, monitor, approval, interaction response, blocker, or terminal disposition. - Top-level issue comments should still wake the assignee on agent-assigned, non-terminal issues. Related PR search performed: - Found related prior document annotation work: #6733. - Found related prior issue-comment wake work and revert context: #7678, #7765. - No existing PR for `PAP-10613-why-is-this-task-not-running`. ## What Changed - Removed the document annotation comment assignee wake helper from issue routes. - Kept document annotation reference sync and activity logging intact. - Documented the distinction between top-level issue comments and document-scoped comments in `doc/execution-semantics.md`. - Added route tests proving document/document annotation activity does not wake the assignee. - Added route coverage proving top-level board issue comments still wake the assignee. ## Verification - `pnpm exec vitest run server/src/__tests__/document-annotation-routes.test.ts server/src/__tests__/issue-update-comment-wakeup-routes.test.ts` — 2 files passed, 9 tests passed. - `pnpm --filter @paperclipai/server typecheck` — passed. - `git status -sb` — clean branch tracking `origin/PAP-10613-why-is-this-task-not-running`. ## Risks - Low to moderate behavior change: document annotation comments no longer wake the issue assignee automatically. - Operators who want document feedback to route work must use an explicit primitive such as assignment, issue-thread comment, agent mention, issue-thread interaction, approval, blocker, or delegated follow-up. - No database migration or public API shape change. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used OpenAI Codex, GPT-5-based coding agent with shell/tool use enabled. Exact hosted runtime model identifier beyond GPT-5 was not exposed in this session. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] If this change affects the UI, I have included before/after screenshots - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
Dotta
@@ -12,6 +12,7 @@ const mockIssueService = vi.hoisted(() => ({
|
||||
}));
|
||||
const mockDocumentService = vi.hoisted(() => ({
|
||||
getIssueDocumentByKey: vi.fn(),
|
||||
upsertIssueDocument: vi.fn(),
|
||||
}));
|
||||
const mockAnnotationService = vi.hoisted(() => ({
|
||||
listThreadsForIssueDocument: vi.fn(),
|
||||
@@ -196,6 +197,15 @@ describe("document annotation routes", () => {
|
||||
});
|
||||
mockIssueService.assertCheckoutOwner.mockResolvedValue({});
|
||||
mockDocumentService.getIssueDocumentByKey.mockResolvedValue(documentPayload);
|
||||
mockDocumentService.upsertIssueDocument.mockResolvedValue({
|
||||
created: false,
|
||||
document: {
|
||||
...documentPayload,
|
||||
body: "Alpha updated selected text omega",
|
||||
latestRevisionId: "99999999-9999-4999-8999-999999999999",
|
||||
latestRevisionNumber: 2,
|
||||
},
|
||||
});
|
||||
mockAnnotationService.listThreadsForIssueDocument.mockImplementation(async (
|
||||
_issueId: string,
|
||||
_key: string,
|
||||
@@ -237,7 +247,35 @@ describe("document annotation routes", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("creates annotation threads, syncs references, logs activity, and wakes the assignee", async () => {
|
||||
it("updates issue documents without waking the assignee through the issue-comment path", async () => {
|
||||
mockIssueService.getById.mockResolvedValue({
|
||||
id: issueId,
|
||||
companyId,
|
||||
title: "Document API",
|
||||
status: "in_progress",
|
||||
assigneeAgentId: "99999999-9999-4999-8999-999999999999",
|
||||
});
|
||||
|
||||
const res = await request(await createApp())
|
||||
.put(`/api/issues/${issueId}/documents/plan`)
|
||||
.send({
|
||||
title: "Plan",
|
||||
format: "markdown",
|
||||
body: "Alpha updated selected text omega",
|
||||
changeSummary: "Document feedback only",
|
||||
baseRevisionId: documentPayload.latestRevisionId,
|
||||
})
|
||||
.expect(200);
|
||||
|
||||
expect(res.body.latestRevisionNumber).toBe(2);
|
||||
expect(mockIssueReferenceService.syncDocument).toHaveBeenCalledWith(documentPayload.id);
|
||||
expect(mockLogActivity).toHaveBeenCalledWith(expect.anything(), expect.objectContaining({
|
||||
action: "issue.document_updated",
|
||||
}));
|
||||
expect(mockHeartbeatService.wakeup).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("creates annotation threads, syncs references, logs activity, and does not wake the assignee", async () => {
|
||||
mockIssueService.getById.mockResolvedValue({
|
||||
id: issueId,
|
||||
companyId,
|
||||
@@ -261,15 +299,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 () => {
|
||||
@@ -278,12 +308,24 @@ describe("document annotation routes", () => {
|
||||
.expect(403);
|
||||
});
|
||||
|
||||
it("adds annotation comments and resolves threads", async () => {
|
||||
it("adds annotation comments without waking the assignee and resolves threads", async () => {
|
||||
mockIssueService.getById.mockResolvedValue({
|
||||
id: issueId,
|
||||
companyId,
|
||||
title: "Annotation API",
|
||||
status: "todo",
|
||||
assigneeAgentId: "99999999-9999-4999-8999-999999999999",
|
||||
});
|
||||
|
||||
await request(await createApp())
|
||||
.post(`/api/issues/${issueId}/documents/plan/annotations/${annotationThread.id}/comments`)
|
||||
.send({ body: "Reply with PAP-2" })
|
||||
.expect(201);
|
||||
expect(mockIssueReferenceService.syncAnnotationComment).toHaveBeenCalledWith(annotationComment.id);
|
||||
expect(mockLogActivity).toHaveBeenCalledWith(expect.anything(), expect.objectContaining({
|
||||
action: "issue.document_annotation_comment_added",
|
||||
}));
|
||||
expect(mockHeartbeatService.wakeup).not.toHaveBeenCalled();
|
||||
|
||||
const resolved = await request(await createApp())
|
||||
.patch(`/api/issues/${issueId}/documents/plan/annotations/${annotationThread.id}`)
|
||||
|
||||
@@ -314,4 +314,48 @@ describe("issue update comment wakeups", () => {
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("wakes the assignee on top-level board issue comments", async () => {
|
||||
const existing = makeIssue({
|
||||
assigneeAgentId: ASSIGNEE_AGENT_ID,
|
||||
assigneeUserId: null,
|
||||
status: "in_progress",
|
||||
});
|
||||
mockIssueService.getById.mockResolvedValue(existing);
|
||||
mockIssueService.addComment.mockResolvedValue({
|
||||
id: "comment-3",
|
||||
issueId: existing.id,
|
||||
companyId: existing.companyId,
|
||||
body: "please handle this top-level thread comment",
|
||||
});
|
||||
|
||||
const res = await request(await createApp())
|
||||
.post(`/api/issues/${existing.id}/comments`)
|
||||
.send({
|
||||
body: "please handle this top-level thread comment",
|
||||
});
|
||||
|
||||
expect(res.status).toBe(201);
|
||||
await vi.waitFor(() => 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-3",
|
||||
mutation: "comment",
|
||||
}),
|
||||
contextSnapshot: expect.objectContaining({
|
||||
issueId: existing.id,
|
||||
taskId: existing.id,
|
||||
commentId: "comment-3",
|
||||
wakeCommentId: "comment-3",
|
||||
wakeReason: "issue_commented",
|
||||
source: "issue.comment",
|
||||
}),
|
||||
}),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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);
|
||||
},
|
||||
);
|
||||
|
||||
Reference in New Issue
Block a user