Files
PaperClipAI/server/src/__tests__/issue-comment-redaction.test.ts
T
1477d1ecea test: remove the no-op sequential describe modifier (#15286)
## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work.
> - Paperclip runs the server and runner test suites with Vitest.
> - Vitest 5 removes the deprecated `describe.sequential` property, so
the pending Vitest 5 upgrade fails the type-check and test jobs.
> - `describe.sequential` only changes behaviour inside a
`describe.concurrent` suite, or when `sequence.concurrent` is on.
> - This repository has neither, so the modifier changed nothing at run
time.
> - The benefit is that the Vitest 5 upgrade can land, and the test
files lose a modifier that did no work.

## Linked Issues or Issue Description

Refs: #12969

## What Changed

- Replace every `describe.sequential` use with a plain `describe` call.
- Drop the `{ concurrent: false }` suite option from the two runner test
files.
- Add a comment to `server/vitest.config.ts` that records why these
suites must run one test at a time.
- Leave the package manifests and the lockfile unchanged.

## Why the modifier did nothing

The Vitest documentation states that `describe.sequential` is useful to
run tests in sequence inside a `describe.concurrent` suite, or with the
`--sequence.concurrent` option. `sequence.concurrent` defaults to
`false`.

This repository satisfies neither condition:

- No test file uses `describe.concurrent`, `it.concurrent`, or
`test.concurrent`.
- `server/vitest.config.ts` sets `sequence.concurrent: false`, with
`maxWorkers: 1`, `maxConcurrency: 1`, and `isolate: true`.
- `packages/paperclip-runner/vitest.config.ts` sets no `sequence` block,
so the `false` default applies.

`packages/db` and `cli` already run the same embedded-Postgres suites
with a plain `describe`, and those jobs are green. The server package
was the only outlier.

The modifier did carry one real piece of knowledge: these suites need
their tests to run one at a time. The new comment in
`server/vitest.config.ts` records that reason next to the setting that
enforces it.

## Verification

- `git grep` for `describe.sequential` returns nothing outside
`node_modules`.
- The author ran the changed server test files under the installed
Vitest 4, and the results match the results without this change.
- Two very large embedded-Postgres test files exceeded the author's
local memory limit, so the CI test jobs cover those two.
- The two changed runner test files have pre-existing local failures
caused by a missing Rust toolchain and a missing global `pnpm` binary.
The failures are identical with and without this change.
- The author type-checked the changed files and found no new error.
- CI must pass the typecheck, build, server test, and runner verify
jobs.

## Risks

- Low risk. Suite execution stays serial, because the Vitest config
enforces it.
- The change adds no dependency and changes no package manifest or
lockfile.
- A future change that turns `sequence.concurrent` on would break these
suites. The new config comment warns against it.

## Model Used

- Claude Sonnet 5 — code edits and local verification.
- OpenAI Codex, GPT-5 — the earlier revision of this branch.

## Test plan

- [x] Every CI check reaches a terminal green state. A pending or queued
check is not a pass.
- [x] The `Typecheck + Release Registry` job passes. This change must
not introduce a type error.
- [x] The `Build` job passes.
- [x] The server test jobs and the runner verify jobs pass.
- [x] Greptile re-reviews this commit set and posts a passing verdict.
The dependabot waiver does not apply to this pull request.
- [x] `mergeable` reads `MERGEABLE` as a terminal value.

## 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 linked the related public issue with `Refs: #12969`
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change and contains no internal
Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [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: Priya Raman <priya.raman@paperclip.ing>

---------

Co-authored-by: Priya Raman <priya.raman@paperclip.ing>
Co-authored-by: Paperclip <noreply@paperclip.ing>
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Co-authored-by: nickyleach <331803+nickyleach@users.noreply.github.com>
2026-10-06 07:19:15 -07:00

583 lines
17 KiB
TypeScript

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 {
assets,
companies,
closeRegisteredClients,
companyMemberships,
createDb,
issueAttachments,
issueComments,
issueReferenceMentions,
issues,
} from "@paperclipai/db";
import {
companySearchQuerySchema,
LOW_TRUST_REVIEW_PRESET,
} 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 externalTestDatabaseUrl = process.env.PAPERCLIP_TEST_DATABASE_URL;
const embeddedPostgresSupport = externalTestDatabaseUrl
? { supported: true }
: await getEmbeddedPostgresTestSupport();
const describeEmbeddedPostgres = embeddedPostgresSupport.supported
? describe
: 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 () => {
if (externalTestDatabaseUrl) {
db = createDb(externalTestDatabaseUrl);
} else {
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(issueAttachments);
await db.delete(issueComments);
await db.delete(assets);
await db.delete(issues);
await db.delete(companyMemberships);
await db.delete(companies);
});
afterAll(async () => {
if (externalTestDatabaseUrl) {
await closeRegisteredClients(externalTestDatabaseUrl);
}
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",
});
await db.insert(companyMemberships).values({
companyId,
principalType: "user",
principalId: "board-user-1",
status: "active",
membershipRole: "owner",
updatedAt: new Date(),
});
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",
// cloud_tenant actors are never instance admins — reads flow through
// the active company membership seeded in seedIssue().
isInstanceAdmin: false,
};
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("never includes a same-company comment from another issue in a wake payload", async () => {
const { companyId, issueId } = await seedIssue();
const foreignIssueId = randomUUID();
await db.insert(issues).values({
id: foreignIssueId,
companyId,
identifier: "RED-2",
title: "Foreign issue",
status: "todo",
priority: "medium",
});
const [sourceComment, foreignComment] = await db
.insert(issueComments)
.values([
{
companyId,
issueId,
authorUserId: "board-user-1",
body: "Full external instruction. TRAILING-CLAUSE: keep this exact requirement.",
},
{
companyId,
issueId: foreignIssueId,
authorUserId: "board-user-1",
body: "FOREIGN-ISSUE-SECRET must never cross the task boundary",
},
])
.returning();
const wakePayload = await buildPaperclipWakePayload({
db,
companyId,
contextSnapshot: {
issueId,
wakeCommentId: sourceComment!.id,
wakeCommentIds: [sourceComment!.id, foreignComment!.id],
wakeReason: "issue_commented",
},
});
expect(wakePayload?.comments).toEqual([
expect.objectContaining({
id: sourceComment!.id,
issueId,
body: "Full external instruction. TRAILING-CLAUSE: keep this exact requirement.",
bodyTruncated: false,
}),
]);
expect(wakePayload?.commentWindow).toEqual({
requestedCount: 2,
includedCount: 1,
missingCount: 1,
});
expect(JSON.stringify(wakePayload)).not.toContain("FOREIGN-ISSUE-SECRET");
});
it("includes bounded attachment descriptors only for requested comments in the same company and issue", async () => {
const { companyId, issueId } = await seedIssue();
const otherIssueId = randomUUID();
const otherCompanyId = randomUUID();
const otherCompanyIssueId = randomUUID();
await db.insert(companies).values({
id: otherCompanyId,
name: "Other Attachment Co",
issuePrefix: "OAT",
requireBoardApprovalForNewAgents: false,
});
await db.insert(issues).values([
{
id: otherIssueId,
companyId,
identifier: "RED-ATT-2",
title: "Other attachment issue",
status: "todo",
priority: "medium",
},
{
id: otherCompanyIssueId,
companyId: otherCompanyId,
identifier: "OAT-1",
title: "Other company attachment issue",
status: "todo",
priority: "medium",
},
]);
const [
sourceComment,
unrequestedComment,
quarantinedComment,
otherIssueComment,
otherCompanyComment,
] = await db
.insert(issueComments)
.values([
{
companyId,
issueId,
authorUserId: "board-user-1",
body: "Inspect the attached evidence.",
},
{
companyId,
issueId,
authorUserId: "board-user-1",
body: "Same task, not part of this wake.",
},
{
companyId,
issueId,
body: "Quarantined file instructions.",
sourceTrust: {
preset: LOW_TRUST_REVIEW_PRESET,
disposition: "quarantined",
sourceIssueId: issueId,
},
},
{
companyId,
issueId: otherIssueId,
authorUserId: "board-user-1",
body: "Other task.",
},
{
companyId: otherCompanyId,
issueId: otherCompanyIssueId,
authorUserId: "other-user",
body: "Other company.",
},
])
.returning();
const attachmentFixtures = [
{
companyId,
issueId,
issueCommentId: sourceComment!.id,
filename: "evidence.png",
},
{
companyId,
issueId,
issueCommentId: unrequestedComment!.id,
filename: "same-task-unrequested.txt",
},
{
companyId,
issueId,
issueCommentId: quarantinedComment!.id,
filename: "quarantined.txt",
},
{
companyId,
issueId: otherIssueId,
issueCommentId: otherIssueComment!.id,
filename: "other-task.txt",
},
{
companyId: otherCompanyId,
issueId: otherCompanyIssueId,
issueCommentId: otherCompanyComment!.id,
filename: "other-company.txt",
},
];
for (const [index, fixture] of attachmentFixtures.entries()) {
const [asset] = await db
.insert(assets)
.values({
companyId: fixture.companyId,
provider: "local_disk",
objectKey: `wake-attachment-${index}`,
contentType: index === 0 ? "image/png" : "text/plain",
byteSize: index === 0 ? 2048 : 128,
sha256: `sha-${index}`,
originalFilename: fixture.filename,
createdByUserId: "board-user-1",
})
.returning();
await db.insert(issueAttachments).values({
companyId: fixture.companyId,
issueId: fixture.issueId,
issueCommentId: fixture.issueCommentId,
assetId: asset!.id,
});
}
const wakePayload = await buildPaperclipWakePayload({
db,
companyId,
contextSnapshot: {
issueId,
wakeCommentIds: [
sourceComment!.id,
sourceComment!.id,
quarantinedComment!.id,
otherIssueComment!.id,
otherCompanyComment!.id,
],
wakeReason: "issue_commented",
},
});
expect(wakePayload?.comments).toEqual(
expect.arrayContaining([
expect.objectContaining({
id: sourceComment!.id,
attachments: [
expect.objectContaining({
filename: "evidence.png",
contentType: "image/png",
byteSize: 2048,
}),
],
}),
expect.objectContaining({ id: quarantinedComment!.id }),
]),
);
expect(
wakePayload?.comments.find(
(comment) => comment.id === quarantinedComment!.id,
),
).not.toHaveProperty("attachments");
expect(JSON.stringify(wakePayload)).not.toContain(
"same-task-unrequested.txt",
);
expect(JSON.stringify(wakePayload)).not.toContain("other-task.txt");
expect(JSON.stringify(wakePayload)).not.toContain("other-company.txt");
expect(JSON.stringify(wakePayload)).not.toContain("quarantined.txt");
const lowTrustWakePayload = await buildPaperclipWakePayload({
db,
companyId,
contextSnapshot: {
issueId,
wakeCommentIds: [quarantinedComment!.id],
wakeReason: "issue_commented",
},
exposeLowTrustRaw: true,
});
expect(lowTrustWakePayload?.comments).toEqual([
expect.objectContaining({
id: quarantinedComment!.id,
attachments: [
expect.objectContaining({ filename: "quarantined.txt" }),
],
}),
]);
});
it("serializes comment timestamps as ISO strings through the redacted comments route (PAP-16607)", async () => {
const { companyId, issueId } = await seedIssue();
const commentId = randomUUID();
await db.insert(issueComments).values({
id: commentId,
companyId,
issueId,
authorUserId: "board-user-1",
body: "ordinary comment",
});
const response = await request(createApp(companyId)).get(
`/api/issues/${issueId}/comments`,
);
expect(response.status, JSON.stringify(response.body)).toBe(200);
expect(response.body).toHaveLength(1);
// Secret redaction must not collapse Date instances to `{}` — the chat
// renderer needs parseable timestamps.
expect(typeof response.body[0].createdAt).toBe("string");
expect(Number.isNaN(new Date(response.body[0].createdAt).getTime())).toBe(
false,
);
expect(typeof response.body[0].updatedAt).toBe("string");
});
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([]);
});
});