diff --git a/doc/execution-semantics.md b/doc/execution-semantics.md index 93c23997..72acb2f4 100644 --- a/doc/execution-semantics.md +++ b/doc/execution-semantics.md @@ -1,7 +1,7 @@ # Execution Semantics Status: Current implementation guide -Date: 2026-05-23 +Date: 2026-06-08 Audience: Product and engineering This document explains how Paperclip interprets issue assignment, issue status, execution runs, wakeups, parent/sub-issue structure, and blocker relationships. @@ -237,6 +237,23 @@ The valid action-path primitives are: - a first-class blocker chain whose unresolved leaf issues are themselves healthy - an open explicit recovery action that names the owner and action needed to restore liveness +### Comment and document activity wake sources + +Issue-thread comments and document-scoped comments have different wake semantics. + +A top-level issue comment created by a board user or other user on an agent-assigned, non-terminal issue may wake that issue's assignee. This is the normal "the owner should see new issue-thread feedback" path, and the wake payload should identify the issue comment that caused the wake when possible. + +Issue document comments, document annotation comments, and document review comments do not wake the issue assignee by default. They remain visible as document activity and should be discoverable from the issue's document/review surfaces, but document activity is not itself an issue execution path. A document comment can provide evidence or context for the next run, but it must not be treated as a queued wake, monitor, approval, interaction response, blocker, or terminal disposition. + +Document-scoped activity may still route work when it is converted into an explicit action-path primitive. Valid routing exceptions include: + +- an issue mention or structured agent mention that intentionally wakes or assigns a named participant +- a document-review assignment that names a reviewer or assignee for the review state +- a response to an issue-thread interaction, such as `request_confirmation`, `ask_user_questions`, or `suggest_tasks` +- intentional board routing that assigns or reassigns the issue, opens a first-class blocker, creates delegated follow-up work, or queues a typed wake + +Freeform document approval text is not auto-acceptance. Plan approval, implementation approval, or review acceptance must flow through the explicit interaction, approval, execution-policy, assignment, or blocker primitives that define who owns the next move. + ### Adapter-backed workspace coherence For adapter-backed execution, an active run or queued wake counts as a live path only when Paperclip can also prove that the selected workspace is coherent for that adapter invocation. A wake that cannot start in the intended workspace is only a failed delivery attempt, not a healthy liveness path. diff --git a/server/src/__tests__/document-annotation-routes.test.ts b/server/src/__tests__/document-annotation-routes.test.ts index f37ae0f8..3e5201d7 100644 --- a/server/src/__tests__/document-annotation-routes.test.ts +++ b/server/src/__tests__/document-annotation-routes.test.ts @@ -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}`) 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 89b1b257..5c1ebe22 100644 --- a/server/src/__tests__/issue-update-comment-wakeup-routes.test.ts +++ b/server/src/__tests__/issue-update-comment-wakeup-routes.test.ts @@ -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", + }), + }), + ); + }); }); diff --git a/server/src/routes/issues.ts b/server/src/routes/issues.ts index 40768a74..6241b79f 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); }, );