mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-04 02:32:07 +01:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Agents use AI connections to run tasks for a responsible user. > - A connection can deny access when its owner has not shared it with that user. > - The task conversation hides this cause behind a collapsed failure marker. > - People need to see which connection blocked the agent and how to fix it. > - This pull request shows an expanded access error in the task conversation. > - The error names the agent and connection, and links to the agent's AI settings. ## Linked Issues or Issue Description **What happened?** A task could fail because its responsible user could not use the agent's AI connection. The user saw a temporary error toast and a collapsed task failure. The task did not clearly explain the access problem. **Expected behavior** The task should show a visible error that names the agent, the connection, and the person who lacks access. For example: "Codie is configured to use Dotta's API Key, but you don't have access to that credential." It should offer a link to change the AI connection. **Steps to reproduce** 1. Configure an agent with a personal AI connection. 2. Start a task for a user who cannot use that connection. 3. Open the task after the run fails. 4. Check that the access error stays visible and links to AI settings. Refs #15714. That change classifies expected connection-sharing denials. This change makes the same failure clear in task chat. ## What Changed - Keep the connection display name in private error metadata when a sharing check denies access. - Store a private grant reference for the name visibility check. Use one rule for task summaries and full run responses. Show the name to the affected user and the connection sharing audience. Other task readers see a generic label. Strip grant IDs from public run responses. - Show an expanded error card for native and legacy task runs. Use "you" only when the viewer is the responsible user. - Link to AI settings and the failed run. Preserve names on resumed native runs. Hide the old error after a newer successful attempt. - Add full task-page Storybook examples, focused tests, and design guidance. ## Verification - Passed 249 focused tests for task chat, error text, private error metadata, and the real database run summary. These include resumed native runs and viewer access rules. - Passed all UI token gates on the rebased branch. - Checked the production task page in Storybook. Verified the viewer's error, another user's error, mobile layout, and a newer successful attempt. - Passed `pnpm -r typecheck` and `pnpm build` in a clean checkout with Node 24 and the pinned pnpm version. Passed the final server typecheck after the response filter changes. - Passed the real database route regression for the connection owner, affected user, and another task reader. Passed the registered secret redaction tests. - The local route checks needed `--testTimeout 60000` for cold imports. A broader local route run hit cold-import timeouts and an old fixture assertion that is now fixed. The duplicate local `pnpm test:run` aggregate was stopped to free resources. The full suite passed in CI with the normal timeouts. - Passed the full [final-commit CI workflow](https://github.com/paperclipai/paperclip/actions/runs/37996148342), including all server, UI, runner, browser, typecheck, build, and canary packaging gates. All 54 executed checks passed; two Storybook jobs were skipped. Greptile reviewed the final commit at 5/5 with no remaining findings, and every review thread is resolved. - Review the Storybook group `Task / Credential access` for the notice and full task-page examples. ## Risks - Only the allowed connection display name enters the compact run summary. Credential secrets, grant IDs, and arbitrary error details do not enter it. - Existing runs can lack the connection name. They show a clear fallback message. - The notice uses the existing task-run relevance rules. A newer attempt or a completed task hides the old failure. - No schema migration or API permission change is required. ## Model Used OpenAI GPT-6 through Codex, with tool execution and browser verification. The exact serving variant and context-window size are 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 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 - [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>
344 lines
11 KiB
TypeScript
344 lines
11 KiB
TypeScript
import express from "express";
|
|
import request from "supertest";
|
|
import { beforeEach, describe, expect, it, vi } from "vitest";
|
|
|
|
const mockActivityService = vi.hoisted(() => ({
|
|
list: vi.fn(),
|
|
forIssue: vi.fn(),
|
|
runsForIssue: vi.fn(),
|
|
issuesForRun: vi.fn(),
|
|
create: vi.fn(),
|
|
}));
|
|
|
|
const mockHeartbeatService = vi.hoisted(() => ({
|
|
getRun: vi.fn(),
|
|
}));
|
|
|
|
const mockIssueService = vi.hoisted(() => ({
|
|
getById: vi.fn(),
|
|
getByIdentifier: vi.fn(),
|
|
}));
|
|
|
|
const mockAccessService = vi.hoisted(() => ({
|
|
decide: vi.fn(),
|
|
canUser: vi.fn(),
|
|
}));
|
|
|
|
const mockAgentActionAuditService = vi.hoisted(() => ({
|
|
list: vi.fn(),
|
|
}));
|
|
|
|
vi.mock("../services/activity.js", () => ({
|
|
activityService: () => mockActivityService,
|
|
normalizeActivityLimit: (limit: number | undefined) => {
|
|
if (!Number.isFinite(limit)) return 100;
|
|
return Math.max(1, Math.min(500, Math.floor(limit ?? 100)));
|
|
},
|
|
}));
|
|
|
|
vi.mock("../services/index.js", () => ({
|
|
accessService: () => mockAccessService,
|
|
issueService: () => mockIssueService,
|
|
heartbeatService: () => mockHeartbeatService,
|
|
}));
|
|
|
|
vi.mock("../services/authorization.js", async (importOriginal) => {
|
|
const actual = await importOriginal<typeof import("../services/authorization.js")>();
|
|
return {
|
|
...actual,
|
|
issueReadSqlCondition: vi.fn(async () => ({}) as any),
|
|
};
|
|
});
|
|
|
|
vi.mock("../services/agent-action-audit.js", () => ({
|
|
agentActionAuditService: () => mockAgentActionAuditService,
|
|
}));
|
|
|
|
async function createApp(
|
|
actor: Record<string, unknown> = {
|
|
type: "board",
|
|
userId: "user-1",
|
|
companyIds: ["company-1"],
|
|
source: "session",
|
|
isInstanceAdmin: false,
|
|
},
|
|
) {
|
|
vi.resetModules();
|
|
const [{ errorHandler }, { activityRoutes }] = await Promise.all([
|
|
import("../middleware/index.js") as Promise<typeof import("../middleware/index.js")>,
|
|
import("../routes/activity.js") as Promise<typeof import("../routes/activity.js")>,
|
|
]);
|
|
const app = express();
|
|
app.use(express.json());
|
|
app.use((req, _res, next) => {
|
|
(req as any).actor = {
|
|
...actor,
|
|
companyIds: Array.isArray(actor.companyIds) ? [...actor.companyIds] : actor.companyIds,
|
|
};
|
|
next();
|
|
});
|
|
app.use("/api", activityRoutes({} as any));
|
|
app.use(errorHandler);
|
|
return app;
|
|
}
|
|
|
|
async function requestApp(
|
|
app: express.Express,
|
|
buildRequest: (baseUrl: string) => request.Test,
|
|
) {
|
|
const { createServer } = await vi.importActual<typeof import("node:http")>("node:http");
|
|
const server = createServer(app);
|
|
try {
|
|
await new Promise<void>((resolve) => {
|
|
server.listen(0, "127.0.0.1", resolve);
|
|
});
|
|
const address = server.address();
|
|
if (!address || typeof address === "string") {
|
|
throw new Error("Expected HTTP server to listen on a TCP port");
|
|
}
|
|
return await buildRequest(`http://127.0.0.1:${address.port}`);
|
|
} finally {
|
|
if (server.listening) {
|
|
await new Promise<void>((resolve, reject) => {
|
|
server.close((error) => {
|
|
if (error) reject(error);
|
|
else resolve();
|
|
});
|
|
});
|
|
}
|
|
}
|
|
}
|
|
|
|
describe("activity routes", () => {
|
|
beforeEach(() => {
|
|
for (const mock of Object.values(mockActivityService)) mock.mockReset();
|
|
for (const mock of Object.values(mockHeartbeatService)) mock.mockReset();
|
|
for (const mock of Object.values(mockIssueService)) mock.mockReset();
|
|
mockAccessService.decide.mockReset();
|
|
mockAccessService.canUser.mockReset();
|
|
mockAgentActionAuditService.list.mockReset();
|
|
mockAccessService.decide.mockResolvedValue({
|
|
allowed: true,
|
|
action: "company_scope:read",
|
|
reason: "allow_test",
|
|
explanation: "Allowed by test mock.",
|
|
});
|
|
mockAccessService.canUser.mockResolvedValue(false);
|
|
});
|
|
|
|
it("returns redacted all-actors rows to a basic company reader", async () => {
|
|
mockAgentActionAuditService.list.mockResolvedValue({
|
|
items: [{
|
|
id: "activity-1",
|
|
companyId: "company-1",
|
|
actorType: "plugin",
|
|
actorId: "plugin-1",
|
|
action: "plugin.synced",
|
|
entityType: "company",
|
|
entityId: "company-1",
|
|
agentId: "agent-1",
|
|
runId: "run-1",
|
|
responsibleUserId: "user-2",
|
|
details: { privateAttribution: true },
|
|
createdAt: "2026-08-04T00:00:00.000Z",
|
|
entity: { issue: null, comment: null, document: null },
|
|
}],
|
|
nextCursor: null,
|
|
});
|
|
|
|
const app = await createApp();
|
|
const res = await request(app)
|
|
.get("/api/companies/company-1/audit/agent-actions?actorScope=all");
|
|
|
|
expect(res.status).toBe(200);
|
|
expect(mockAgentActionAuditService.list).toHaveBeenCalledWith({
|
|
companyId: "company-1",
|
|
actorScope: "all",
|
|
limit: 50,
|
|
});
|
|
expect(res.body.items[0]).toMatchObject({
|
|
actorType: "plugin",
|
|
actorId: "plugin-1",
|
|
action: "plugin.synced",
|
|
entityType: "company",
|
|
entityId: "company-1",
|
|
createdAt: "2026-08-04T00:00:00.000Z",
|
|
agentId: null,
|
|
runId: null,
|
|
responsibleUserId: null,
|
|
details: null,
|
|
});
|
|
expect(res.body.accessTier).toBe("basic");
|
|
});
|
|
|
|
it("rejects attribution filters for a basic all-actors reader", async () => {
|
|
const app = await createApp();
|
|
const res = await request(app)
|
|
.get("/api/companies/company-1/audit/agent-actions?actorScope=all&runId=00000000-0000-4000-8000-000000000001");
|
|
|
|
expect(res.status).toBe(403);
|
|
expect(res.body.error).toContain("audit:view_agent_actions");
|
|
expect(mockAgentActionAuditService.list).not.toHaveBeenCalled();
|
|
});
|
|
|
|
it("keeps attribution for a permitted all-actors reader", async () => {
|
|
mockAccessService.canUser.mockResolvedValue(true);
|
|
mockAgentActionAuditService.list.mockResolvedValue({
|
|
items: [{
|
|
id: "activity-1",
|
|
agentId: "agent-1",
|
|
runId: "run-1",
|
|
responsibleUserId: "user-2",
|
|
details: { attribution: true },
|
|
}],
|
|
nextCursor: null,
|
|
});
|
|
|
|
const app = await createApp();
|
|
const res = await request(app)
|
|
.get("/api/companies/company-1/audit/agent-actions?actorScope=all&actorType=system");
|
|
|
|
expect(res.status).toBe(200);
|
|
expect(mockAgentActionAuditService.list).toHaveBeenCalledWith({
|
|
companyId: "company-1",
|
|
actorScope: "all",
|
|
actorType: "system",
|
|
limit: 50,
|
|
});
|
|
expect(res.body.items[0]).toMatchObject({
|
|
agentId: "agent-1",
|
|
runId: "run-1",
|
|
responsibleUserId: "user-2",
|
|
details: { attribution: true },
|
|
});
|
|
expect(res.body.accessTier).toBe("full");
|
|
});
|
|
|
|
it("limits company activity lists by default", async () => {
|
|
mockActivityService.list.mockResolvedValue([]);
|
|
|
|
const app = await createApp();
|
|
const res = await requestApp(app, (baseUrl) => request(baseUrl).get("/api/companies/company-1/activity"));
|
|
|
|
expect(res.status).toBe(200);
|
|
expect(mockActivityService.list).toHaveBeenCalledWith({
|
|
companyId: "company-1",
|
|
agentId: undefined,
|
|
entityType: undefined,
|
|
entityId: undefined,
|
|
limit: 100,
|
|
readCondition: expect.anything(),
|
|
});
|
|
});
|
|
|
|
it("caps requested company activity list limits", async () => {
|
|
mockActivityService.list.mockResolvedValue([]);
|
|
|
|
const app = await createApp();
|
|
const res = await requestApp(app, (baseUrl) =>
|
|
request(baseUrl).get("/api/companies/company-1/activity?limit=5000&entityType=issue"),
|
|
);
|
|
|
|
expect(res.status).toBe(200);
|
|
expect(mockActivityService.list).toHaveBeenCalledWith({
|
|
companyId: "company-1",
|
|
agentId: undefined,
|
|
entityType: "issue",
|
|
entityId: undefined,
|
|
limit: 500,
|
|
readCondition: expect.anything(),
|
|
});
|
|
});
|
|
|
|
it("resolves alphanumeric issue identifiers before loading runs", async () => {
|
|
mockIssueService.getByIdentifier.mockResolvedValue({
|
|
id: "issue-uuid-1",
|
|
companyId: "company-1",
|
|
});
|
|
mockActivityService.runsForIssue.mockResolvedValue([
|
|
{
|
|
runId: "run-1",
|
|
adapterType: "codex_local",
|
|
},
|
|
]);
|
|
|
|
const app = await createApp();
|
|
const res = await requestApp(app, (baseUrl) => request(baseUrl).get("/api/issues/pc1a2-475/runs"));
|
|
|
|
expect(res.status).toBe(200);
|
|
expect(mockIssueService.getByIdentifier).toHaveBeenCalledWith("PC1A2-475");
|
|
expect(mockIssueService.getById).not.toHaveBeenCalled();
|
|
expect(mockActivityService.runsForIssue).toHaveBeenCalledWith("company-1", "issue-uuid-1", "user-1");
|
|
expect(res.body).toEqual([{ runId: "run-1", adapterType: "codex_local" }]);
|
|
});
|
|
|
|
it("requires company access before creating activity events", async () => {
|
|
const app = await createApp();
|
|
const res = await requestApp(app, (baseUrl) => request(baseUrl)
|
|
.post("/api/companies/company-2/activity")
|
|
.send({
|
|
actorId: "user-1",
|
|
action: "test.event",
|
|
entityType: "issue",
|
|
entityId: "issue-1",
|
|
}));
|
|
|
|
expect(res.status).toBe(403);
|
|
expect(mockActivityService.create).not.toHaveBeenCalled();
|
|
});
|
|
|
|
it.each([null, { id: "issue-uuid-1", companyId: "company-2" }])(
|
|
"does not start run reads or retries for a missing or cross-company task", async (issue) => {
|
|
mockIssueService.getByIdentifier.mockResolvedValue(issue);
|
|
const app = await createApp();
|
|
const res = await request(app).get("/api/issues/TEST-1/runs");
|
|
expect(res.status).toBe(404);
|
|
expect(res.body).toEqual({ error: "Issue not found" });
|
|
expect(mockActivityService.runsForIssue).not.toHaveBeenCalled();
|
|
},
|
|
);
|
|
|
|
it("does not start run reads or retries when task read authorization is denied", async () => {
|
|
mockIssueService.getByIdentifier.mockResolvedValue({ id: "issue-uuid-1", companyId: "company-1" });
|
|
mockAccessService.decide.mockResolvedValue({ allowed: false, reason: "denied", explanation: "Denied by test policy." });
|
|
const app = await createApp();
|
|
const res = await request(app).get("/api/issues/TEST-1/runs");
|
|
expect(res.status).toBe(403);
|
|
expect(mockActivityService.runsForIssue).not.toHaveBeenCalled();
|
|
});
|
|
|
|
it("returns 200 [] (not 404) when listing issues for another company's run, preserving API contract and the cross-tenant oracle", async () => {
|
|
mockHeartbeatService.getRun.mockResolvedValue({
|
|
id: "run-2",
|
|
companyId: "company-2",
|
|
});
|
|
|
|
const app = await createApp();
|
|
const res = await requestApp(app, (baseUrl) => request(baseUrl).get("/api/heartbeat-runs/run-2/issues"));
|
|
|
|
expect(res.status).toBe(200);
|
|
expect(res.body).toEqual([]);
|
|
expect(mockActivityService.issuesForRun).not.toHaveBeenCalled();
|
|
});
|
|
|
|
it("returns 200 [] (not 404) for a non-existent heartbeat run, matching the cross-tenant response", async () => {
|
|
mockHeartbeatService.getRun.mockResolvedValue(null);
|
|
|
|
const app = await createApp();
|
|
const res = await requestApp(app, (baseUrl) => request(baseUrl).get("/api/heartbeat-runs/missing-run/issues"));
|
|
|
|
expect(res.status).toBe(200);
|
|
expect(res.body).toEqual([]);
|
|
expect(mockActivityService.issuesForRun).not.toHaveBeenCalled();
|
|
});
|
|
|
|
it("rejects anonymous heartbeat run issue lookups before run existence checks", async () => {
|
|
const app = await createApp({ type: "none", source: "none" });
|
|
const res = await requestApp(app, (baseUrl) => request(baseUrl).get("/api/heartbeat-runs/missing-run/issues"));
|
|
|
|
expect(res.status).toBe(401);
|
|
expect(mockHeartbeatService.getRun).not.toHaveBeenCalled();
|
|
expect(mockActivityService.issuesForRun).not.toHaveBeenCalled();
|
|
});
|
|
});
|