Merge pull request #7554 from paperclipai/codex/pap-10343-comment-redaction

[codex] Redact deleted issue comments
This commit is contained in:
Dotta
2026-06-05 05:32:08 -10:00
committed by GitHub
30 changed files with 40631 additions and 98 deletions
@@ -9,6 +9,7 @@ import {
documentAnnotationThreads,
documentRevisions,
documents,
issueComments,
issueDocuments,
issues,
} from "@paperclipai/db";
@@ -58,6 +59,7 @@ describeEmbeddedPostgres("documentAnnotationService", () => {
await db.delete(documentRevisions);
await db.delete(issueDocuments);
await db.delete(documents);
await db.delete(issueComments);
await db.delete(issues);
await db.delete(companies);
});
@@ -180,4 +182,136 @@ describeEmbeddedPostgres("documentAnnotationService", () => {
const threads = await db.select().from(documentAnnotationThreads);
expect(threads).toHaveLength(0);
});
it("removes linked annotation comments and resolves empty threads when an issue comment is deleted", async () => {
const { companyId, issueId, document } = await createIssueWithDocument();
const [issueComment] = await db
.insert(issueComments)
.values({
companyId,
issueId,
authorType: "user",
authorUserId: "board-user",
body: "Delete this linked comment",
})
.returning();
const thread = await annotations.createThread(
issueId,
"plan",
{
baseRevisionId: document.latestRevisionId!,
baseRevisionNumber: document.latestRevisionNumber,
selector: {
quote: { exact: "selected text", prefix: "Alpha ", suffix: " omega" },
position: { normalizedStart: 6, normalizedEnd: 19, markdownStart: 6, markdownEnd: 19 },
},
body: "Linked annotation body",
issueCommentId: issueComment.id,
},
{ actorType: "user", actorId: "board-user", userId: "board-user" },
);
const cleanup = await annotations.cleanupForIssueCommentDeletion(
issueId,
issueComment.id,
{ actorType: "user", actorId: "board-user", userId: "board-user" },
);
expect(cleanup.deletedCommentIds).toEqual([thread.comments[0]!.id]);
expect(cleanup.resolvedThreadIds).toEqual([thread.id]);
await expect(
db.select().from(documentAnnotationComments).where(eq(documentAnnotationComments.id, thread.comments[0]!.id)),
).resolves.toHaveLength(0);
const [updatedThread] = await db
.select()
.from(documentAnnotationThreads)
.where(eq(documentAnnotationThreads.id, thread.id));
expect(updatedThread?.status).toBe("resolved");
expect(updatedThread?.resolvedByUserId).toBe("board-user");
});
it("rejects annotation comments linked to already-deleted issue comments", async () => {
const { companyId, issueId, document } = await createIssueWithDocument();
const [issueComment] = await db
.insert(issueComments)
.values({
companyId,
issueId,
authorType: "user",
authorUserId: "board-user",
body: "",
deletedAt: new Date("2026-06-05T03:00:00.000Z"),
deletedByType: "user",
deletedByUserId: "board-user",
})
.returning();
await expect(
annotations.createThread(
issueId,
"plan",
{
baseRevisionId: document.latestRevisionId!,
baseRevisionNumber: document.latestRevisionNumber,
selector: {
quote: { exact: "selected text", prefix: "Alpha ", suffix: " omega" },
position: { normalizedStart: 6, normalizedEnd: 19, markdownStart: 6, markdownEnd: 19 },
},
body: "Do not link this annotation to a deleted comment",
issueCommentId: issueComment.id,
},
{ actorType: "user", actorId: "board-user", userId: "board-user" },
),
).rejects.toMatchObject({
status: 422,
message: "Linked issue comment must belong to this issue",
});
await expect(db.select().from(documentAnnotationComments)).resolves.toHaveLength(0);
});
it("does not report already-resolved empty threads as newly resolved during linked comment cleanup", async () => {
const { companyId, issueId, document } = await createIssueWithDocument();
const [issueComment] = await db
.insert(issueComments)
.values({
companyId,
issueId,
authorType: "user",
authorUserId: "board-user",
body: "Delete this linked comment from a resolved thread",
})
.returning();
const thread = await annotations.createThread(
issueId,
"plan",
{
baseRevisionId: document.latestRevisionId!,
baseRevisionNumber: document.latestRevisionNumber,
selector: {
quote: { exact: "selected text", prefix: "Alpha ", suffix: " omega" },
position: { normalizedStart: 6, normalizedEnd: 19, markdownStart: 6, markdownEnd: 19 },
},
body: "Linked annotation body",
issueCommentId: issueComment.id,
},
{ actorType: "user", actorId: "board-user", userId: "board-user" },
);
await db
.update(documentAnnotationThreads)
.set({ status: "resolved", resolvedByUserId: "board-user", resolvedAt: new Date("2026-06-05T03:05:00.000Z") })
.where(eq(documentAnnotationThreads.id, thread.id));
const cleanup = await annotations.cleanupForIssueCommentDeletion(
issueId,
issueComment.id,
{ actorType: "user", actorId: "board-user", userId: "board-user" },
);
expect(cleanup.deletedCommentIds).toEqual([thread.comments[0]!.id]);
expect(cleanup.resolvedThreadIds).toEqual([]);
});
});
@@ -7,6 +7,7 @@ const mockIssueService = vi.hoisted(() => ({
assertCheckoutOwner: vi.fn(),
getComment: vi.fn(),
removeComment: vi.fn(),
tombstoneComment: vi.fn(),
}));
const mockAccessService = vi.hoisted(() => ({
@@ -38,6 +39,24 @@ const mockIssueThreadInteractionService = vi.hoisted(() => ({
expireRequestConfirmationsSupersededByComment: vi.fn(async () => []),
expireStaleRequestConfirmationsForIssueDocument: vi.fn(async () => []),
}));
const mockDocumentAnnotationService = vi.hoisted(() => ({
cleanupForIssueCommentDeletion: vi.fn(async () => ({ deletedCommentIds: [], resolvedThreadIds: [] })),
remapOpenThreadsForDocument: vi.fn(async () => []),
}));
const mockIssueReferenceService = vi.hoisted(() => ({
deleteCommentSource: vi.fn(async () => undefined),
deleteDocumentSource: vi.fn(async () => undefined),
diffIssueReferenceSummary: vi.fn(() => ({
addedReferencedIssues: [],
removedReferencedIssues: [],
currentReferencedIssues: [],
})),
emptySummary: vi.fn(() => ({ outbound: [], inbound: [] })),
listIssueReferenceSummary: vi.fn(async () => ({ outbound: [], inbound: [] })),
syncComment: vi.fn(async () => undefined),
syncDocument: vi.fn(async () => undefined),
syncIssue: vi.fn(async () => undefined),
}));
function registerModuleMocks() {
vi.doMock("@paperclipai/shared/telemetry", () => ({
@@ -79,7 +98,7 @@ function registerModuleMocks() {
}),
accessService: () => mockAccessService,
agentService: () => ({ getById: vi.fn(async () => null) }),
documentAnnotationService: () => ({ remapOpenThreadsForDocument: async () => [] }),
documentAnnotationService: () => mockDocumentAnnotationService,
documentService: () => ({}),
executionWorkspaceService: () => ({}),
feedbackService: () => mockFeedbackService,
@@ -91,19 +110,7 @@ function registerModuleMocks() {
getActiveForIssue: vi.fn(async () => null),
listActiveForIssues: vi.fn(async () => new Map()),
}),
issueReferenceService: () => ({
deleteDocumentSource: async () => undefined,
diffIssueReferenceSummary: () => ({
addedReferencedIssues: [],
removedReferencedIssues: [],
currentReferencedIssues: [],
}),
emptySummary: () => ({ outbound: [], inbound: [] }),
listIssueReferenceSummary: async () => ({ outbound: [], inbound: [] }),
syncComment: async () => undefined,
syncDocument: async () => undefined,
syncIssue: async () => undefined,
}),
issueReferenceService: () => mockIssueReferenceService,
issueService: () => mockIssueService,
issueThreadInteractionService: () => mockIssueThreadInteractionService,
logActivity: mockLogActivity,
@@ -188,6 +195,19 @@ describe.sequential("issue comment cancel routes", () => {
mockIssueService.assertCheckoutOwner.mockResolvedValue({ adoptedFromRunId: null });
mockIssueService.getComment.mockResolvedValue(makeComment());
mockIssueService.removeComment.mockResolvedValue(makeComment());
mockIssueService.tombstoneComment.mockImplementation(async (_commentId, _actor, options) => {
const deleted = makeComment({
body: "",
metadata: null,
deletedAt: new Date("2026-04-11T15:05:00.000Z"),
deletedByType: "user",
deletedByAgentId: null,
deletedByUserId: "local-board",
deletedByRunId: null,
});
await options?.afterTombstone?.(deleted, "tx");
return deleted;
});
mockAccessService.canUser.mockResolvedValue(false);
mockAccessService.hasPermission.mockResolvedValue(false);
mockFeedbackService.listIssueVotesForUser.mockResolvedValue([]);
@@ -214,13 +234,23 @@ describe.sequential("issue comment cancel routes", () => {
});
mockInstanceSettingsService.listCompanyIds.mockResolvedValue(["company-1"]);
mockLogActivity.mockResolvedValue(undefined);
mockDocumentAnnotationService.cleanupForIssueCommentDeletion.mockResolvedValue({
deletedCommentIds: [],
resolvedThreadIds: [],
});
mockIssueReferenceService.deleteCommentSource.mockResolvedValue(undefined);
mockIssueReferenceService.syncComment.mockResolvedValue(undefined);
});
it("cancels a queued comment from its author and restores the deleted body", async () => {
const res = await request(await installActor(createApp()))
.delete("/api/issues/11111111-1111-4111-8111-111111111111/comments/comment-1");
.delete("/api/issues/11111111-1111-4111-8111-111111111111/comments/comment-1?mode=cancel");
expect(res.status).toBe(200);
expect(res.status, JSON.stringify({
body: res.body,
tombstoneCalls: mockIssueService.tombstoneComment.mock.calls,
activityCalls: mockLogActivity.mock.calls,
})).toBe(200);
expect(res.body).toMatchObject({
id: "comment-1",
body: "Queued follow-up",
@@ -239,6 +269,18 @@ describe.sequential("issue comment cancel routes", () => {
);
});
it("rejects stale queued cancellation after the active run is gone", async () => {
mockHeartbeatService.getRun.mockResolvedValue(null);
const res = await request(await installActor(createApp()))
.delete("/api/issues/11111111-1111-4111-8111-111111111111/comments/comment-1?mode=cancel");
expect(res.status).toBe(409);
expect(res.body.error).toBe("Queued comment can no longer be canceled");
expect(mockIssueService.removeComment).not.toHaveBeenCalled();
expect(mockIssueService.tombstoneComment).not.toHaveBeenCalled();
});
it("rejects canceling comments that are no longer queued", async () => {
mockIssueService.getComment.mockResolvedValue(
makeComment({
@@ -248,11 +290,12 @@ describe.sequential("issue comment cancel routes", () => {
);
const res = await request(await installActor(createApp()))
.delete("/api/issues/11111111-1111-4111-8111-111111111111/comments/comment-1");
.delete("/api/issues/11111111-1111-4111-8111-111111111111/comments/comment-1?mode=cancel");
expect(res.status).toBe(409);
expect(res.body.error).toBe("Only queued comments can be canceled");
expect(mockIssueService.removeComment).not.toHaveBeenCalled();
expect(mockIssueService.tombstoneComment).not.toHaveBeenCalled();
});
it("rejects canceling another actor's queued comment", async () => {
@@ -263,10 +306,92 @@ describe.sequential("issue comment cancel routes", () => {
);
const res = await request(await installActor(createApp()))
.delete("/api/issues/11111111-1111-4111-8111-111111111111/comments/comment-1");
.delete("/api/issues/11111111-1111-4111-8111-111111111111/comments/comment-1?mode=cancel");
expect(res.status).toBe(403);
expect(res.body.error).toBe("Only the comment author can cancel queued comments");
expect(mockIssueService.removeComment).not.toHaveBeenCalled();
});
it("deletes a normal authored comment as a tombstone without returning the original body", async () => {
mockIssueService.getComment.mockResolvedValue(
makeComment({
body: "Sensitive original comment body",
metadata: { version: 1, sections: [{ rows: [{ type: "text", text: "Sensitive metadata copy" }] }] },
createdAt: new Date("2026-04-11T14:58:00.000Z"),
updatedAt: new Date("2026-04-11T14:58:00.000Z"),
}),
);
mockDocumentAnnotationService.cleanupForIssueCommentDeletion.mockResolvedValue({
deletedCommentIds: ["annotation-comment-1"],
resolvedThreadIds: ["annotation-thread-1"],
});
const res = await request(await installActor(createApp()))
.delete("/api/issues/11111111-1111-4111-8111-111111111111/comments/comment-1");
expect(res.status, JSON.stringify(res.body)).toBe(200);
expect(res.body).toMatchObject({
id: "comment-1",
body: "",
metadata: null,
deletedByType: "user",
deletedByUserId: "local-board",
});
expect(JSON.stringify(res.body)).not.toContain("Sensitive original comment body");
expect(JSON.stringify(res.body)).not.toContain("Sensitive metadata copy");
expect(mockIssueService.removeComment).not.toHaveBeenCalled();
expect(mockIssueService.tombstoneComment).toHaveBeenCalledWith(
"comment-1",
{
actorType: "user",
agentId: null,
userId: "local-board",
runId: null,
},
expect.objectContaining({ afterTombstone: expect.any(Function) }),
);
expect(mockIssueReferenceService.syncComment).toHaveBeenCalledWith("comment-1", "tx");
expect(mockDocumentAnnotationService.cleanupForIssueCommentDeletion).toHaveBeenCalledWith(
"11111111-1111-4111-8111-111111111111",
"comment-1",
expect.objectContaining({
actorType: "user",
userId: "local-board",
}),
"tx",
);
expect(mockIssueReferenceService.deleteCommentSource).toHaveBeenCalledWith("annotation-comment-1", "tx");
const deletedActivity = mockLogActivity.mock.calls.find((call) => call[1]?.action === "issue.comment_deleted")?.[1];
expect(deletedActivity).toEqual(expect.objectContaining({
action: "issue.comment_deleted",
details: expect.objectContaining({
deletedAnnotationCommentIds: ["annotation-comment-1"],
resolvedAnnotationThreadIds: ["annotation-thread-1"],
}),
}));
expect(deletedActivity?.details).toEqual(expect.not.objectContaining({
bodySnippet: expect.anything(),
}));
expect(JSON.stringify(deletedActivity?.details ?? {})).not.toContain("Sensitive original comment body");
expect(JSON.stringify(deletedActivity?.details ?? {})).not.toContain("Sensitive metadata copy");
});
it("rejects deleting another actor's normal comment", async () => {
mockIssueService.getComment.mockResolvedValue(
makeComment({
authorUserId: "someone-else",
createdAt: new Date("2026-04-11T14:58:00.000Z"),
updatedAt: new Date("2026-04-11T14:58:00.000Z"),
}),
);
const res = await request(await installActor(createApp()))
.delete("/api/issues/11111111-1111-4111-8111-111111111111/comments/comment-1");
expect(res.status).toBe(403);
expect(res.body.error).toBe("Only the comment author can delete comments");
expect(mockIssueService.removeComment).not.toHaveBeenCalled();
expect(mockIssueService.tombstoneComment).not.toHaveBeenCalled();
});
});
@@ -0,0 +1,248 @@
import { randomUUID } from "node:crypto";
import express from "express";
import request from "supertest";
import { sql } from "drizzle-orm";
import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "vitest";
import {
companies,
createDb,
issueComments,
issueReferenceMentions,
issues,
} from "@paperclipai/db";
import { companySearchQuerySchema } from "@paperclipai/shared";
import {
getEmbeddedPostgresTestSupport,
startEmbeddedPostgresTestDatabase,
} from "./helpers/embedded-postgres.js";
import { errorHandler } from "../middleware/index.js";
import { issueRoutes } from "../routes/issues.js";
import { companySearchService } from "../services/company-search.js";
import { buildPaperclipWakePayload } from "../services/heartbeat.js";
import { issueReferenceService } from "../services/issue-references.js";
import { issueService } from "../services/issues.js";
import type { StorageService } from "../storage/types.js";
const embeddedPostgresSupport = await getEmbeddedPostgresTestSupport();
const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe.sequential : describe.skip;
if (!embeddedPostgresSupport.supported) {
console.warn(
`Skipping embedded Postgres issue comment redaction tests on this host: ${embeddedPostgresSupport.reason ?? "unsupported environment"}`,
);
}
describeEmbeddedPostgres("deleted issue comment redaction", () => {
let db!: ReturnType<typeof createDb>;
let tempDb: Awaited<ReturnType<typeof startEmbeddedPostgresTestDatabase>> | null = null;
beforeAll(async () => {
tempDb = await startEmbeddedPostgresTestDatabase("paperclip-comment-redaction-");
db = createDb(tempDb.connectionString);
await db.execute(sql.raw("CREATE EXTENSION IF NOT EXISTS pg_trgm"));
}, 20_000);
afterEach(async () => {
await db.delete(issueReferenceMentions);
await db.delete(issueComments);
await db.delete(issues);
await db.delete(companies);
});
afterAll(async () => {
await tempDb?.cleanup();
});
async function seedIssue() {
const companyId = randomUUID();
const issueId = randomUUID();
await db.insert(companies).values({
id: companyId,
name: "Comment Redaction Co",
issuePrefix: `R${companyId.replace(/-/g, "").slice(0, 6).toUpperCase()}`,
requireBoardApprovalForNewAgents: false,
});
await db.insert(issues).values({
id: issueId,
companyId,
identifier: "RED-1",
title: "Deleted comment redaction",
status: "todo",
priority: "medium",
});
return { companyId, issueId };
}
function createApp(companyId: string) {
const app = express();
app.use(express.json());
app.use((req, _res, next) => {
(req as any).actor = {
type: "board",
userId: "board-user-1",
companyIds: [companyId],
memberships: [{ companyId, membershipRole: "owner", status: "active" }],
source: "cloud_tenant",
isInstanceAdmin: true,
};
next();
});
const storage: StorageService = {
provider: "local_disk",
putFile: vi.fn(async () => {
throw new Error("Unexpected storage.putFile call");
}),
getObject: vi.fn(async () => {
throw new Error("Unexpected storage.getObject call");
}),
headObject: vi.fn(async () => ({ exists: false })),
deleteObject: vi.fn(async () => undefined),
};
app.use("/api", issueRoutes(db, storage));
app.use(errorHandler);
return app;
}
it("redacts deleted comment bodies from ordinary reads, heartbeat context, and wake payloads", async () => {
const { companyId, issueId } = await seedIssue();
const commentId = randomUUID();
const deletedAt = new Date("2026-06-03T12:00:00.000Z");
await db.insert(issueComments).values({
id: commentId,
companyId,
issueId,
authorUserId: "board-user-1",
body: "secret deleted body",
presentation: { kind: "system_notice", tone: "warning" },
metadata: { version: 1, sections: [{ rows: [{ type: "text", text: "secret metadata" }] }] },
deletedAt,
deletedByType: "user",
deletedByUserId: "board-user-1",
});
const comments = await issueService(db).listComments(issueId, { order: "asc" });
expect(comments).toHaveLength(1);
expect(comments[0]).toMatchObject({
id: commentId,
body: "",
presentation: null,
metadata: null,
deletedAt,
deletedByType: "user",
deletedByUserId: "board-user-1",
});
const exactComment = await issueService(db).getComment(commentId);
expect(exactComment?.body).toBe("");
expect(exactComment?.metadata).toBeNull();
const heartbeatContext = await request(createApp(companyId))
.get(`/api/issues/${issueId}/heartbeat-context`)
.query({ wakeCommentId: commentId });
expect(heartbeatContext.status, JSON.stringify(heartbeatContext.body)).toBe(200);
expect(heartbeatContext.body.wakeComment).toMatchObject({
id: commentId,
body: "",
metadata: null,
deletedByUserId: "board-user-1",
});
expect(JSON.stringify(heartbeatContext.body)).not.toContain("secret deleted body");
expect(JSON.stringify(heartbeatContext.body)).not.toContain("secret metadata");
const wakePayload = await buildPaperclipWakePayload({
db,
companyId,
contextSnapshot: {
issueId,
commentId,
wakeCommentIds: [commentId],
wakeReason: "issue_commented",
},
});
expect(wakePayload?.comments).toEqual([
expect.objectContaining({
id: commentId,
body: "",
bodyTruncated: false,
presentation: null,
metadata: null,
deletedAt: deletedAt.toISOString(),
deletedByUserId: "board-user-1",
}),
]);
expect(JSON.stringify(wakePayload)).not.toContain("secret deleted body");
expect(JSON.stringify(wakePayload)).not.toContain("secret metadata");
});
it("excludes deleted comment bodies from company search", async () => {
const { companyId, issueId } = await seedIssue();
await db.insert(issueComments).values({
companyId,
issueId,
body: "vanished-search-needle",
deletedAt: new Date("2026-06-03T12:00:00.000Z"),
deletedByType: "user",
deletedByUserId: "board-user-1",
});
const result = await companySearchService(db).search(
companyId,
companySearchQuerySchema.parse({ q: "vanished-search-needle", scope: "comments" }),
);
expect(result.results).toEqual([]);
});
it("clears issue references sourced from deleted comment bodies", async () => {
const companyId = randomUUID();
const sourceIssueId = randomUUID();
const targetIssueId = randomUUID();
const commentId = randomUUID();
await db.insert(companies).values({
id: companyId,
name: "Reference Redaction Co",
issuePrefix: "REF",
requireBoardApprovalForNewAgents: false,
});
await db.insert(issues).values([
{
id: sourceIssueId,
companyId,
identifier: "REF-1",
title: "Source issue",
status: "todo",
priority: "medium",
},
{
id: targetIssueId,
companyId,
identifier: "REF-2",
title: "Target issue",
status: "todo",
priority: "medium",
},
]);
await db.insert(issueComments).values({
id: commentId,
companyId,
issueId: sourceIssueId,
body: "Follow up in REF-2",
});
const refs = issueReferenceService(db);
await refs.syncComment(commentId);
expect((await refs.listIssueReferenceSummary(sourceIssueId)).outbound.map((item) => item.issue.id)).toEqual([
targetIssueId,
]);
await db.update(issueComments).set({
deletedAt: new Date("2026-06-03T12:00:00.000Z"),
deletedByType: "user",
deletedByUserId: "board-user-1",
});
await refs.syncComment(commentId);
expect((await refs.listIssueReferenceSummary(sourceIssueId)).outbound).toEqual([]);
});
});
@@ -107,4 +107,57 @@ describe("plugin SDK test harness", () => {
"missing required capability 'authorization.audit.read'",
);
});
it("returns tombstone-safe deleted comments from the in-memory issue helper", async () => {
const manifest: PaperclipPluginManifestV1 = {
id: "paperclip.test-comment-redaction",
apiVersion: 1,
version: "0.1.0",
displayName: "Comment Redaction",
description: "Test plugin",
author: "Paperclip",
categories: ["automation"],
capabilities: ["issue.comments.read"],
entrypoints: { worker: "./dist/worker.js" },
};
const harness = createTestHarness({ manifest });
harness.seed({
issues: [{
id: "issue-1",
companyId: "company-1",
title: "Comment redaction",
status: "todo",
priority: "medium",
}],
issueComments: [{
id: "comment-1",
companyId: "company-1",
issueId: "issue-1",
authorType: "user",
authorAgentId: null,
authorUserId: "user-1",
body: "secret plugin-visible body",
presentation: { kind: "system_notice", tone: "warning" },
metadata: { version: 1, sections: [{ rows: [{ type: "text", text: "secret plugin metadata" }] }] },
deletedAt: new Date("2026-06-03T12:00:00.000Z"),
deletedByType: "user",
deletedByUserId: "user-1",
createdAt: new Date("2026-06-03T11:00:00.000Z"),
updatedAt: new Date("2026-06-03T12:00:00.000Z"),
}],
});
const comments = await harness.ctx.issues.listComments("issue-1", "company-1");
expect(comments).toHaveLength(1);
expect(comments[0]).toMatchObject({
id: "comment-1",
body: "",
presentation: null,
metadata: null,
deletedByUserId: "user-1",
});
expect(JSON.stringify(comments)).not.toContain("secret plugin-visible body");
expect(JSON.stringify(comments)).not.toContain("secret plugin metadata");
});
});