Files
PaperClipAI/server/src/__tests__/agent-commentary-reader.test.ts
T
DottaandClaude Opus 5.5 00ad9503fe feat(commentary): agents read only their own commentary unless granted commentary:view (#19842)
## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work
> - Agents can submit complaints and suggestions as commentary, and
people with commentary access read them on the Commentary page
> - When a person asks an agent in chat what agents have been
complaining about, the agent cannot read commentary and gets "Board
access required"
> - That response does not say where a person can see it, so the agent
can name the feed but cannot link it
> - Product rule: an agent cannot read other agents' commentary unless
it holds `commentary:view`, which is off by default; it may read its own
> - This pull request implements that rule and adds a remediation to the
denial naming the human page for the agent's own company
> - The benefit is a clear permission boundary and a working link for
the person instead of a dead end

## Linked Issues or Issue Description

**What happened?**
In the `activity.agent-feedback` chat eval, the agent correctly said it
could not read agent commentary and that only the person could open it,
but it gave no link, because the denial named no page.

**Expected behavior**
An agent denied a commentary read learns which page a person with
commentary access can open.

**Steps to reproduce**
1. As an agent, call `GET /api/companies/{companyId}/agent-commentary`.
2. Before this change, the 403 body is only `{ "error": "Board access
required" }`.

## What Changed

- `server/src/routes/agent-commentary.ts`:
- An agent with the `commentary:view` grant reads all commentary its
task access allows. The grant is not among the default agent grants.
- Without the grant, an agent may read its own entries: `?agentId=<its
own id>` on the list, or its own entry by ID.
- Without the grant, listing all commentary, filtering by another agent,
or reading another agent's entry returns 403 "Board access required".
The response's `remediation` names `/<issuePrefix>/activity/commentary`
for the agent's own company.
- A request for another company still fails on company access first,
with no page path.
- Board reads are unchanged: `commentary:view` for the user, plus task
read conditions.
- `agent-commentary.integration.test.ts`: one test covers the full-list
and other-agent denials with the page path, own-list and own-entry
reads, the cross-company denial, the grant allowing a full list, and
that `commentary:view` is not a default agent grant.

## Verification

- `npx vitest run src/__tests__/agent-commentary.integration.test.ts` in
`server`: 21 tests passed.
- The new test fails against the previous route.
- Negative control: with the previous route, the new test fails.
- Related commentary tests: `agent-commentary-reader`,
`agent-commentary-helper`, `commentary-permission-migration` passed.
`agent-commentary-skills` fails the same way with and without this
change (a missing skill file in a temporary directory locally).
- The chat eval rerun with this change is recorded in #19614.

## Risks

- Agents can now read their own commentary when they ask for it
explicitly; before, every agent read was denied.
- The denial's status code and message are unchanged; a `remediation`
string is added for same-company agents.
- There is no UI yet for granting `commentary:view` to an agent.
- An agent learns the company's issue prefix, which it already sees in
task identifiers.

> 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

- Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, with tool use and code
execution in Claude Code.

## 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 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 (e.g. `docs/...`, `fix/...`)
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
- [ ] All Paperclip CI gates are green
- [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [x] I will address all Greptile and reviewer comments before
requesting merge

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
2026-11-07 12:04:46 -06:00

221 lines
14 KiB
TypeScript

import { randomUUID } from "node:crypto";
import { sql, eq } from "drizzle-orm";
import express from "express";
import request from "supertest";
import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "vitest";
import { agentCommentary, agents, authUsers, companies, companyMemberships, createDb, heartbeatRuns, issueAccessGrants, issues, principalPermissionGrants } from "@paperclipai/db";
import { agentCommentaryQuerySchema } from "@paperclipai/shared";
import { startEmbeddedPostgresTestDatabase } from "./helpers/embedded-postgres.js";
import { agentCommentaryReader } from "../services/agent-commentary-reader.js";
import { agentCommentaryRoutes } from "../routes/agent-commentary.js";
import { errorHandler } from "../middleware/error-handler.js";
import { accessService } from "../services/access.js";
import { grantsForHumanRole } from "../services/company-member-roles.js";
describe("agent commentary browsing", () => {
let temporary: Awaited<ReturnType<typeof startEmbeddedPostgresTestDatabase>>;
let db: ReturnType<typeof createDb>;
let reader: ReturnType<typeof agentCommentaryReader>;
const companyId = randomUUID(), foreignCompanyId = randomUUID();
const firstAgentId = randomUUID(), secondAgentId = randomUUID();
const visibleTaskId = randomUUID(), privateTaskId = randomUUID();
const firstId = randomUUID(), privateId = randomUUID(), foreignId = randomUUID();
const longBody = "Preserve evidence history. " + "A useful observation. ".repeat(100);
const query = (values: object = {}) => agentCommentaryQuerySchema.parse(values);
beforeAll(async () => {
temporary = await startEmbeddedPostgresTestDatabase("paperclip-commentary-reader-");
db = createDb(temporary.connectionString);
reader = agentCommentaryReader(db);
await db.insert(companies).values([
{ id: companyId, name: "Feedback fixtures", issuePrefix: "FDB" },
{ id: foreignCompanyId, name: "Other company", issuePrefix: "OTH" },
]);
await db.insert(agents).values([
{ id: firstAgentId, companyId, name: "Writer", role: "engineer", adapterType: "codex_local" },
{ id: secondAgentId, companyId, name: "Reviewer", role: "engineer", adapterType: "claude_local" },
{ id: foreignCompanyId, companyId: foreignCompanyId, name: "Other writer", role: "engineer", adapterType: "codex_local" },
]);
await db.insert(issues).values([
{ id: visibleTaskId, companyId, title: "Visible task", identifier: "FDB-1" },
{ id: privateTaskId, companyId, title: "Private task", identifier: "FDB-2" },
]);
const runId = randomUUID(), foreignRunId = randomUUID();
await db.insert(heartbeatRuns).values([
{ id: runId, companyId, agentId: firstAgentId, status: "succeeded" },
{ id: foreignRunId, companyId: foreignCompanyId, agentId: foreignCompanyId, status: "succeeded" },
]);
await db.insert(agentCommentary).values([
{ id: firstId, companyId, agentId: firstAgentId, runId, issueId: visibleTaskId, kind: "suggestion", body: longBody, idempotencyKey: "first", payloadHash: "hash", createdAt: new Date("2026-01-03T12:00:00Z") },
{ id: randomUUID(), companyId, agentId: secondAgentId, runId, issueId: visibleTaskId, kind: "complaint", body: "Installer failed at 100% completion with literal_under_score.", idempotencyKey: "second", payloadHash: "hash", createdAt: new Date("2026-01-02T12:00:00Z") },
{ id: privateId, companyId, agentId: firstAgentId, runId, issueId: privateTaskId, kind: "suggestion", body: "Private task feedback", idempotencyKey: "private", payloadHash: "hash", createdAt: new Date("2026-01-01T12:00:00Z") },
{ id: foreignId, companyId: foreignCompanyId, agentId: foreignCompanyId, runId: foreignRunId, kind: "complaint", body: "Other company feedback", idempotencyKey: "foreign", payloadHash: "hash" },
]);
}, 90_000);
afterAll(async () => { await temporary?.cleanup(); });
afterEach(() => vi.unstubAllEnvs());
it("bounds previews and pagination while details retain the full message", async () => {
const page = await reader.list(companyId, query({ limit: 1 }), sql`true`);
expect(page).toMatchObject({ total: 3, limit: 1, offset: 0 });
expect(page.items[0]).toMatchObject({ id: firstId, agent: { name: "Writer" }, task: { identifier: "FDB-1" } });
expect(page.items[0].preview).toHaveLength(280);
expect(page.items[0]).not.toHaveProperty("body");
expect((await reader.get(companyId, firstId, sql`true`))?.body).toBe(longBody);
expect((await reader.list(companyId, query({ limit: 1, offset: 1 }), sql`true`)).items[0].kind).toBe("complaint");
});
it("loads 50 messages by default and retrieves the remaining page without duplicates", async () => {
const pagedCompanyId = randomUUID(), agentId = randomUUID(), runId = randomUUID();
await db.insert(companies).values({ id: pagedCompanyId, name: "Pagination fixtures", issuePrefix: "PAG" });
await db.insert(agents).values({ id: agentId, companyId: pagedCompanyId, name: "Writer", role: "engineer", adapterType: "codex_local" });
await db.insert(heartbeatRuns).values({ id: runId, companyId: pagedCompanyId, agentId, status: "succeeded" });
await db.insert(agentCommentary).values(Array.from({ length: 58 }, (_, index) => ({
companyId: pagedCompanyId, agentId, runId, kind: "suggestion" as const, body: `Message ${index}`,
idempotencyKey: String(index), payloadHash: "hash",
})));
const first = await reader.list(pagedCompanyId, query(), sql`true`);
const second = await reader.list(pagedCompanyId, query({ offset: 50 }), sql`true`);
expect(first).toMatchObject({ total: 58, limit: 50, offset: 0 });
expect(first.items).toHaveLength(50);
expect(second).toMatchObject({ total: 58, limit: 50, offset: 50 });
expect(second.items).toHaveLength(8);
expect(new Set([...first.items, ...second.items].map((entry) => entry.id)).size).toBe(58);
});
it("combines filters and searches literal SQL wildcard characters", async () => {
expect((await reader.list(companyId, query({ agentId: firstAgentId, kind: "complaint" }), sql`true`)).total).toBe(0);
expect((await reader.list(companyId, query({ kind: "complaint", q: "100%" }), sql`true`)).total).toBe(1);
expect((await reader.list(companyId, query({ q: "literal_under_score" }), sql`true`)).total).toBe(1);
expect((await reader.list(companyId, query({ q: "nothing_here" }), sql`true`)).total).toBe(0);
});
it("applies source-task visibility to counts, previews and details and isolates companies", async () => {
const page = await reader.list(companyId, query(), eq(issues.id, visibleTaskId));
expect(page.total).toBe(2);
expect(page.items.map((item) => item.id)).not.toContain(privateId);
expect(await reader.get(companyId, privateId, eq(issues.id, visibleTaskId))).toBeNull();
expect(await reader.get(companyId, foreignId, sql`true`)).toBeNull();
expect((await reader.list(companyId, query(), sql`true`)).items.map((item) => item.id)).not.toContain(foreignId);
});
it.each(["legacy", "native"])("keeps %s feedback hidden after its source task is deleted", async (transport) => {
const taskId = randomUUID(), runId = randomUUID(), id = randomUUID();
await db.insert(issues).values({ id: taskId, companyId, title: "Deleted private task" });
await db.insert(heartbeatRuns).values({
id: runId, companyId, agentId: firstAgentId, status: "succeeded",
...(transport === "native" ? { nativeIssueId: taskId } : { contextSnapshot: { issueId: taskId } }),
});
await db.insert(agentCommentary).values({
id, companyId, agentId: firstAgentId, runId, issueId: taskId, kind: "complaint",
body: "Deleted private task feedback", idempotencyKey: "deleted", payloadHash: "hash",
});
await db.delete(issues).where(eq(issues.id, taskId));
expect(await reader.get(companyId, id, eq(issues.id, visibleTaskId))).toBeNull();
expect((await reader.list(companyId, query({ q: "Deleted private task feedback" }), eq(issues.id, visibleTaskId))).total).toBe(0);
});
it("enforces board membership, commentary permission and filter bounds at the HTTP boundary", async () => {
let actor: Record<string, unknown> = { type: "board", source: "local_implicit", userId: "operator" };
const app = express();
app.use((req, _res, next) => { req.actor = actor as typeof req.actor; next(); });
app.use("/api", agentCommentaryRoutes(db));
app.use(errorHandler);
const url = `/api/companies/${companyId}/agent-commentary`;
expect((await request(app).get(url)).body.total).toBe(3);
expect((await request(app).get(`${url}/${firstId}`)).body.body).toBe(longBody);
expect((await request(app).get(`${url}/${foreignId}`)).status).toBe(404);
for (const filters of ["limit=101", "offset=-1", "kind=other", "agentId=bad-id"]) {
expect((await request(app).get(`${url}?${filters}`)).status).toBe(400);
}
// Agents read only their own commentary unless granted commentary:view.
actor = { type: "agent", companyId, agentId: firstAgentId };
expect((await request(app).get(url)).status).toBe(403);
expect((await request(app).get(`${url}/${firstId}`)).status).toBe(200);
actor = { type: "agent", companyId, agentId: secondAgentId };
expect((await request(app).get(`${url}/${firstId}`)).status).toBe(403);
await db.insert(companyMemberships).values({ companyId, principalType: "agent", principalId: secondAgentId, status: "active", membershipRole: "member" }).onConflictDoNothing();
await db.insert(principalPermissionGrants).values({ companyId, principalType: "agent", principalId: secondAgentId, permissionKey: "commentary:view" });
try {
expect((await request(app).get(`${url}/${firstId}`)).status).toBe(200);
expect((await request(app).get(url)).status).toBe(200);
} finally {
await db.delete(principalPermissionGrants).where(eq(principalPermissionGrants.principalId, secondAgentId));
await db.delete(companyMemberships).where(eq(companyMemberships.principalId, secondAgentId));
}
actor = { type: "board", source: "session", userId: "viewer", companyIds: [companyId] };
const denied = await request(app).get(url).expect(403);
expect(denied.body.error).toContain("commentary:view");
await request(app).get(`${url}/${firstId}`).expect(403);
actor = { type: "board", source: "session", userId: "operator", isInstanceAdmin: true, companyIds: [foreignCompanyId] };
expect((await request(app).get(url)).status).toBe(403);
});
it.each(["owner", "admin", "operator", "viewer"] as const)("lets a default %s read commentary independently of audit access", async (role) => {
const userId = randomUUID();
const access = accessService(db);
await access.ensureMembership(companyId, "user", userId, role);
await access.setPrincipalGrants(companyId, "user", userId, grantsForHumanRole(role), null);
const app = express();
app.use((req, _res, next) => {
req.actor = { type: "board", source: "session", userId, companyIds: [companyId] };
next();
});
app.use("/api", agentCommentaryRoutes(db));
app.use(errorHandler);
const url = `/api/companies/${companyId}/agent-commentary`;
expect((await request(app).get(url).expect(200)).body.total).toBe(3);
expect((await request(app).get(`${url}/${firstId}`).expect(200)).body.body).toBe(longBody);
await expect(access.canUser(companyId, userId, "audit:view_agent_actions")).resolves.toBe(role === "operator");
await request(app).get(`/api/companies/${foreignCompanyId}/agent-commentary`).expect(403);
await request(app).get(`${url}/${foreignId}`).expect(404);
await access.setPrincipalGrants(companyId, "user", userId, [
{ permissionKey: "audit:view_agent_actions", scope: null },
], null);
expect((await request(app).get(url).expect(403)).body.error).toContain("commentary:view");
await request(app).get(`${url}/${firstId}`).expect(403);
});
it("applies actual task privacy to HTTP previews, counts, searches and details for commentary-authorized board users", async () => {
vi.stubEnv("PAPERCLIP_ISSUE_PRIVACY_MODE", "enforce");
const userId = randomUUID(), ownerId = randomUUID();
await db.insert(authUsers).values([userId, ownerId].map((id) => ({
id, name: "Board user", email: `${id}@example.test`, emailVerified: true,
createdAt: new Date(), updatedAt: new Date(),
})));
await db.insert(companyMemberships).values({
companyId, principalType: "user", principalId: userId, status: "active", membershipRole: "viewer",
});
await db.insert(principalPermissionGrants).values({
companyId, principalType: "user", principalId: userId, permissionKey: "commentary:view",
});
await db.update(issues).set({ visibility: "private", privacyRootIssueId: privateTaskId, responsibleUserId: ownerId }).where(eq(issues.id, privateTaskId));
const app = express();
app.use((req, _res, next) => {
req.actor = { type: "board", source: "session", userId, companyIds: [companyId] };
next();
});
app.use("/api", agentCommentaryRoutes(db));
app.use(errorHandler);
const url = `/api/companies/${companyId}/agent-commentary`;
const deniedPage = await request(app).get(url).expect(200);
expect(deniedPage.body.total).toBe(2);
expect(deniedPage.body.items.map((entry: { id: string }) => entry.id)).not.toContain(privateId);
expect((await request(app).get(url).query({ q: "Private task feedback" }).expect(200)).body).toMatchObject({ total: 0, items: [] });
await request(app).get(`${url}/${privateId}`).expect(404);
const [grant] = await db.insert(issueAccessGrants).values({
issueId: privateTaskId, subjectType: "user", subjectId: userId, source: "explicit", grantedByUserId: ownerId,
}).returning();
const grantedPage = await request(app).get(url).expect(200);
expect(grantedPage.body.total).toBe(3);
expect(grantedPage.body.items.map((entry: { id: string }) => entry.id)).toContain(privateId);
expect((await request(app).get(`${url}/${privateId}`).expect(200)).body.body).toBe("Private task feedback");
await db.update(issueAccessGrants).set({ revokedAt: new Date() }).where(eq(issueAccessGrants.id, grant.id));
expect((await request(app).get(url).expect(200)).body.total).toBe(2);
await request(app).get(`${url}/${privateId}`).expect(404);
});
});