PAP-10440: group artifacts by task stacks (#7654)

## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - The artifacts surface is where board users inspect files, media, and documents produced by agents. > - Grouped artifact stacks make that surface easier to scan by task, but the first pass still made grouping feel secondary to media filters. > - The follow-up request was to make grouping the default and give the grouping control the same icon-only outline treatment used on the issues page. > - This pull request keeps the existing artifact grouping API/UI, then polishes the artifacts toolbar state and Storybook review coverage. > - The benefit is that `/artifacts` now opens in the task-stack view by default while preserving explicit flat-mode filtering via `groupBy=none`. ## Linked Issues or Issue Description No public GitHub issue exists for this internal Paperclip task. ### Subsystem affected ui/ — React + Vite board UI. ### Problem or motivation The `/artifacts` grouping affordance was visually placed after the media filters, rendered as a text button, and defaulted to a flat artifact list. Internal follow-up `PAP-10465` requested the grouping icon move left of the filters, become an icon-only outlined button like `/issues`, and make Task grouping the default. ### Proposed solution Default `/artifacts` to grouped Task stacks, keep explicit flat mode available as `groupBy=none`, move the grouping control before the media chips, and restyle it as the shared icon-only outline button pattern. ### Alternatives considered Leaving flat mode as the implicit default was rejected because it does not satisfy the follow-up. Keeping a text label on the grouping trigger was rejected because `/issues` already established the icon-only outline pattern for this class of toolbar control. ### Roadmap alignment This aligns with the `Artifacts & Work Products` roadmap item by making generated outputs easier to inspect and operate from the board UI. ## What Changed - Defaulted the `/artifacts` page to `groupBy=task` when no grouping URL param is present, while keeping explicit flat mode available with `groupBy=none`. - Moved the group control before the media filter chips and changed it to an icon-only outlined button using the shared `Button` pattern. - Updated artifact page tests to cover default Task grouping, explicit flat mode, trigger ordering, and icon-only outline metadata. - Updated the artifact Storybook story so its toolbar mock matches the production ordering and grouped Task is documented as the default mode. ## Verification - `pnpm exec vitest run ui/src/pages/Artifacts.test.tsx ui/src/components/artifacts/ArtifactGroupCard.test.tsx` — passed. - `pnpm --filter @paperclipai/ui typecheck` — passed. - `pnpm --filter @paperclipai/server typecheck` — passed. - `git diff --check` — passed. - QA visual validation from internal follow-up PAP-10466 passed desktop/mobile scenarios. Screenshot evidence attached there: - Desktop default: http://paperclip-dev:3100/api/attachments/bc81305d-f5de-485c-abeb-9e7c3d9d8539/content - Desktop toolbar close-up: http://paperclip-dev:3100/api/attachments/3375a62b-2110-48f3-bafa-ea98c00f99f7/content - Mobile default: http://paperclip-dev:3100/api/attachments/bfc5642e-9248-431e-9bac-36284dec1c89/content - Mobile toolbar close-up: http://paperclip-dev:3100/api/attachments/ca79401a-5ba8-464d-bc6e-aeffd47fe695/content - GitHub PR checks on head `431964c8b` — passed, including Greptile 5/5. ## Risks Low to medium risk. The main behavior shift is intentional: `/artifacts` now queries grouped Task stacks by default. Existing flat mode remains available through the grouping menu and explicit `groupBy=none` URLs. ## Model Used OpenAI Codex, GPT-5.4 class coding model in this Paperclip heartbeat environment, with shell, git, test, and GitHub CLI tool use. Context window managed by the Codex runtime. ## 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 run tests locally and they pass - [x] I have added or updated tests where applicable - [x] If this change affects the UI, I have included before/after screenshots - [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>
2026-06-06 10:22:47 -05:00
parent 2e74d32871
commit d8e1004551
17 changed files with 2011 additions and 114 deletions
@@ -1,5 +1,6 @@
 import { Readable } from "node:stream";
 import express from "express";
+import { eq } from "drizzle-orm";
 import request from "supertest";
 import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "vitest";
 import {
@@ -329,7 +330,7 @@ describeEmbeddedPostgres("companyArtifactsService", () => {
      updatedAt: new Date("2026-01-02T12:00:00.000Z"),
    });

-    return { companyId, projectId, issueId, otherRunId };
+    return { companyId, otherCompanyId, projectId, issueId, secondIssueId, otherIssueId, otherRunId };
  }

  it("projects agent-created documents, direct attachments, and work products while excluding noisy sources", async () => {
@@ -495,6 +496,279 @@ describeEmbeddedPostgres("companyArtifactsService", () => {
    expect(forged?.createdByAgent).toBeNull();
    expect(result.artifacts.some((artifact) => artifact.createdByAgent?.name === "Other")).toBe(false);
  });
+
+  it("does not leak foreign issue or project metadata through malformed artifact link rows", async () => {
+    const { companyId, otherCompanyId, otherIssueId } = await seedArtifacts();
+    const foreignProjectId = "1b1b1b1b-1b1b-4b1b-8b1b-1b1b1b1b1b1b";
+    const malformedAttachmentId = "1c1c1c1c-1c1c-4c1c-8c1c-1c1c1c1c1c1c";
+
+    await db.insert(projects).values({
+      id: foreignProjectId,
+      companyId: otherCompanyId,
+      name: "Foreign Project",
+      status: "in_progress",
+    });
+    await db.update(issues).set({ projectId: foreignProjectId }).where(eq(issues.id, otherIssueId));
+    await db.insert(documents).values({
+      id: "1d1d1d1d-1d1d-4d1d-8d1d-1d1d1d1d1d1d",
+      companyId,
+      title: "Forged Link Document",
+      latestBody: "This row is company-owned but points at a foreign issue.",
+      createdByAgentId: "33333333-3333-4333-8333-333333333333",
+      updatedAt: new Date("2026-01-30T00:00:00.000Z"),
+    });
+    await db.insert(issueDocuments).values({
+      companyId,
+      issueId: otherIssueId,
+      documentId: "1d1d1d1d-1d1d-4d1d-8d1d-1d1d1d1d1d1d",
+      key: "forged-link-document",
+    });
+    await db.insert(assets).values({
+      id: "1e1e1e1e-1e1e-4e1e-8e1e-1e1e1e1e1e1e",
+      companyId,
+      provider: "local_disk",
+      objectKey: "forged-link.txt",
+      contentType: "text/plain",
+      byteSize: 42,
+      sha256: "sha256-forged-link",
+      originalFilename: "forged-link.txt",
+      createdByAgentId: "33333333-3333-4333-8333-333333333333",
+    });
+    await db.insert(issueAttachments).values({
+      id: malformedAttachmentId,
+      companyId,
+      issueId: otherIssueId,
+      assetId: "1e1e1e1e-1e1e-4e1e-8e1e-1e1e1e1e1e1e",
+      updatedAt: new Date("2026-01-29T00:00:00.000Z"),
+    });
+    await db.insert(issueWorkProducts).values({
+      id: "1f1f1f1f-1f1f-4f1f-8f1f-1f1f1f1f1f1f",
+      companyId,
+      issueId: otherIssueId,
+      type: "artifact",
+      provider: "paperclip",
+      title: "Forged Link Work Product",
+      status: "ready_for_review",
+      summary: "This row is company-owned but points at a foreign issue.",
+      metadata: { contentType: "text/plain" },
+      createdByRunId: "99999999-9999-4999-8999-999999999999",
+      updatedAt: new Date("2026-01-28T00:00:00.000Z"),
+    });
+
+    const flat = await companyArtifactsService(db, createStorageService()).list(companyId, { limit: 20 });
+    expect(flat.artifacts.map((artifact) => artifact.title)).not.toEqual(expect.arrayContaining([
+      "Forged Link Document",
+      "forged-link.txt",
+      "Forged Link Work Product",
+    ]));
+    expect(flat.artifacts.some((artifact) => artifact.issue.identifier === "OTH-1")).toBe(false);
+    expect(flat.artifacts.some((artifact) => artifact.issue.title === "Other output")).toBe(false);
+    expect(flat.artifacts.some((artifact) => artifact.project?.name === "Foreign Project")).toBe(false);
+
+    const grouped = await companyArtifactsService(db, createStorageService()).list(companyId, {
+      groupBy: "task",
+      limit: 20,
+    });
+    expect(grouped.groups?.some((group) => group.issue.identifier === "OTH-1")).toBe(false);
+    expect(grouped.groups?.some((group) => group.issue.title === "Other output")).toBe(false);
+    expect(grouped.groups?.some((group) =>
+      group.previewArtifacts.some((artifact) => artifact.project?.name === "Foreign Project")
+    )).toBe(false);
+
+    const selectedForeignGroup = await companyArtifactsService(db, createStorageService()).list(companyId, {
+      groupBy: "task",
+      groupIssueId: otherIssueId,
+      limit: 20,
+    });
+    expect(selectedForeignGroup).toEqual({
+      artifacts: [],
+      selectedGroup: null,
+      nextCursor: null,
+    });
+  });
+
+  it("groups artifacts by task after applying media, project, and search filters", async () => {
+    const { companyId, projectId, issueId } = await seedArtifacts();
+    const storage = createStorageService({ "notes.txt": Buffer.from("Searchable notes preview") });
+
+    const grouped = await companyArtifactsService(db, storage).list(companyId, {
+      groupBy: "task",
+      limit: 10,
+    });
+    expect(grouped.artifacts).toEqual([]);
+    expect(grouped.nextCursor).toBeNull();
+    expect(grouped.groups?.map((group) => ({
+      issue: group.issue.identifier,
+      count: group.count,
+      mediaKinds: group.mediaKinds,
+      href: group.href,
+    }))).toEqual([
+      {
+        issue: "PAP-2",
+        count: 1,
+        mediaKinds: ["document"],
+        href: "/PAP/artifacts?groupBy=task&groupIssueId=77777777-7777-4777-8777-777777777777",
+      },
+      {
+        issue: "PAP-1",
+        count: 3,
+        mediaKinds: ["video", "text"],
+        href: "/PAP/artifacts?groupBy=task&groupIssueId=66666666-6666-4666-8666-666666666666",
+      },
+    ]);
+    expect(grouped.groups?.find((group) => group.issue.id === issueId)?.previewArtifacts.map((artifact) => artifact.title))
+      .toEqual(["direct-video.mp4", "Primary Cut", "notes.txt"]);
+
+    const projectVideos = await companyArtifactsService(db, storage).list(companyId, {
+      groupBy: "task",
+      projectId,
+      kind: "video",
+      limit: 10,
+    });
+    expect(projectVideos.groups?.map((group) => ({
+      issue: group.issue.identifier,
+      count: group.count,
+      href: group.href,
+    }))).toEqual([
+      {
+        issue: "PAP-1",
+        count: 2,
+        href:
+          "/PAP/artifacts?groupBy=task&groupIssueId=66666666-6666-4666-8666-666666666666&kind=video&projectId=55555555-5555-4555-8555-555555555555",
+      },
+    ]);
+
+    const search = await companyArtifactsService(db, storage).list(companyId, {
+      groupBy: "task",
+      q: "review document",
+      limit: 10,
+    });
+    expect(search.groups?.map((group) => ({ issue: group.issue.identifier, count: group.count }))).toEqual([
+      { issue: "PAP-2", count: 1 },
+    ]);
+  });
+
+  it("paginates grouped task lists with the active group cursor", async () => {
+    const { companyId } = await seedArtifacts();
+
+    const firstPage = await companyArtifactsService(db, createStorageService()).list(companyId, {
+      groupBy: "task",
+      limit: 1,
+    });
+    expect(firstPage.groups?.map((group) => group.issue.identifier)).toEqual(["PAP-2"]);
+    expect(firstPage.nextCursor).toEqual(expect.any(String));
+
+    const secondPage = await companyArtifactsService(db, createStorageService()).list(companyId, {
+      groupBy: "task",
+      limit: 10,
+      cursor: firstPage.nextCursor ?? undefined,
+    });
+    expect(secondPage.groups?.map((group) => group.issue.identifier)).toEqual(["PAP-1"]);
+    expect(secondPage.nextCursor).toBeNull();
+  });
+
+  it("groups parent-task artifacts under the topmost same-company ancestor", async () => {
+    const { companyId, issueId, secondIssueId } = await seedArtifacts();
+    const grandchildIssueId = "21212121-2121-4212-8121-212121212121";
+    const grandchildAttachmentId = "23232323-2323-4232-8232-232323232323";
+
+    await db.update(issues).set({ parentId: issueId }).where(eq(issues.id, secondIssueId));
+    await db.insert(issues).values({
+      id: grandchildIssueId,
+      companyId,
+      parentId: secondIssueId,
+      identifier: "PAP-3",
+      title: "Grandchild render",
+      status: "done",
+      priority: "medium",
+    });
+    await db.insert(assets).values({
+      id: "24242424-2424-4242-8242-242424242424",
+      companyId,
+      provider: "local_disk",
+      objectKey: "grandchild.txt",
+      contentType: "text/plain",
+      byteSize: 48,
+      sha256: "sha256-grandchild",
+      originalFilename: "grandchild.txt",
+      createdByAgentId: "33333333-3333-4333-8333-333333333333",
+    });
+    await db.insert(issueAttachments).values({
+      id: grandchildAttachmentId,
+      companyId,
+      issueId: grandchildIssueId,
+      assetId: "24242424-2424-4242-8242-242424242424",
+      updatedAt: new Date("2026-01-05T00:00:00.000Z"),
+    });
+
+    const grouped = await companyArtifactsService(db, createStorageService()).list(companyId, {
+      groupBy: "parent_task",
+      limit: 10,
+    });
+
+    expect(grouped.artifacts).toEqual([]);
+    expect(grouped.groups?.map((group) => ({
+      issue: group.issue.identifier,
+      count: group.count,
+      previewTitles: group.previewArtifacts.map((artifact) => artifact.title),
+    }))).toEqual([
+      {
+        issue: "PAP-1",
+        count: 5,
+        previewTitles: ["grandchild.txt", "Review Notes", "direct-video.mp4"],
+      },
+    ]);
+  });
+
+  it("returns selected group artifact pages and metadata without leaking foreign group issues", async () => {
+    const { companyId, issueId, otherIssueId } = await seedArtifacts();
+
+    const selected = await companyArtifactsService(db, createStorageService()).list(companyId, {
+      groupBy: "task",
+      groupIssueId: issueId,
+      limit: 2,
+    });
+    expect(selected.groups).toBeUndefined();
+    expect(selected.selectedGroup).toMatchObject({
+      id: `task:${issueId}`,
+      groupBy: "task",
+      issue: { identifier: "PAP-1" },
+      count: 3,
+    });
+    expect(selected.artifacts.map((artifact) => artifact.title)).toEqual(["direct-video.mp4", "Primary Cut"]);
+    expect(selected.nextCursor).toEqual(expect.any(String));
+
+    const selectedSecondPage = await companyArtifactsService(db, createStorageService()).list(companyId, {
+      groupBy: "task",
+      groupIssueId: issueId,
+      limit: 10,
+      cursor: selected.nextCursor ?? undefined,
+    });
+    expect(selectedSecondPage.artifacts.map((artifact) => artifact.title)).toEqual(["notes.txt"]);
+
+    const selectedEmptyByFilter = await companyArtifactsService(db, createStorageService()).list(companyId, {
+      groupBy: "task",
+      groupIssueId: issueId,
+      q: "does-not-match-this-stack",
+      limit: 10,
+    });
+    expect(selectedEmptyByFilter.selectedGroup).toMatchObject({
+      id: `task:${issueId}`,
+      count: 0,
+    });
+    expect(selectedEmptyByFilter.artifacts).toEqual([]);
+
+    const foreignSelected = await companyArtifactsService(db, createStorageService()).list(companyId, {
+      groupBy: "task",
+      groupIssueId: otherIssueId,
+      limit: 10,
+    });
+    expect(foreignSelected).toEqual({
+      artifacts: [],
+      selectedGroup: null,
+      nextCursor: null,
+    });
+  });
 });

 describe("company artifacts route authorization", () => {