From d13aeb36f40bdf878f8cf4861c60f4d1687d9ddf Mon Sep 17 00:00:00 2001 From: Dotta Date: Sun, 27 Sep 2026 16:54:25 -0500 Subject: [PATCH] fix(ui): preserve review and download actions for text artifacts Keep Markdown review documents available alongside explicit attachment tabs. Preserve direct card downloads and separate the preview action. Regressions fail with the text context enabled before the fix; 199 focused tests and both desktop/mobile browser cases pass afterward. Co-Authored-By: Paperclip --- doc/AGENT-ARTIFACTS.md | 4 ++ tests/e2e/text-attachment-tabs.spec.ts | 29 +++++++++++++ .../IssuePropertiesArtifactsTab.tsx | 21 ++++++---- ...ssuePropertiesMarkdownWorkProduct.test.tsx | 41 ++++++++++++++++++- .../task-chat/RichWorkProductCard.tsx | 13 +++++- 5 files changed, 97 insertions(+), 11 deletions(-) diff --git a/doc/AGENT-ARTIFACTS.md b/doc/AGENT-ARTIFACTS.md index 157200c97f3..a1e4a170a52 100644 --- a/doc/AGENT-ARTIFACTS.md +++ b/doc/AGENT-ARTIFACTS.md @@ -209,6 +209,10 @@ card, or a chat attachment chip to open a named right-side tab. Reopening the same attachment focuses its existing tab. Tabs can be switched and closed. On mobile, the same viewer opens in the task details drawer. +Markdown work products keep their expandable review document, annotations, +revision indicator, and document links. **Open in tab** is a separate action. +Work-product cards keep **Download** as a direct original-file download. + Markdown attachments offer **Rendered** and **Raw** views. Other supported text files display literal text. The viewer provides a download action. Preview reads are limited to 512 KiB; oversized, unsupported, or unavailable files show an diff --git a/tests/e2e/text-attachment-tabs.spec.ts b/tests/e2e/text-attachment-tabs.spec.ts index cd56c2cddfe..98a11d60eca 100644 --- a/tests/e2e/text-attachment-tabs.spec.ts +++ b/tests/e2e/text-attachment-tabs.spec.ts @@ -23,6 +23,18 @@ for (const mobile of [false, true]) { await json(await request.post(`/api/issues/${issue.id}/comments`, { data: { body: "Review the attached files.", attachmentIds: attachments.map((attachment) => attachment.id) }, })); + for (const [index, attachment] of attachments.entries()) { + await json(await request.post(`/api/issues/${issue.id}/work-products`, { + data: { + type: "artifact", provider: "paperclip", title: `Delivered ${files[index]!.name}`, status: "ready_for_review", + metadata: { + attachmentId: attachment.id, contentType: attachment.contentType, byteSize: attachment.byteSize, + originalFilename: files[index]!.name, contentPath: attachment.contentPath, + openPath: attachment.contentPath, downloadPath: `${attachment.contentPath}?download=1`, + }, + }, + })); + } await page.goto(`/${company.issuePrefix}/issues/${issue.identifier}`); await page.getByRole("link", { name: "Open AGENTS.md", exact: true }).click(); const panel = mobile ? page.getByTestId("mobile-task-side-panel") : page.locator("aside").filter({ has: page.getByRole("tab", { name: "AGENTS.md", exact: true }) }); @@ -50,5 +62,22 @@ for (const mobile of [false, true]) { await panel.getByRole("tab", { name: "AGENTS.md", exact: true }).click(); await expect(panel.getByRole("heading", { name: "File charter", exact: true })).toBeVisible(); await expect(panel.getByRole("tab", { name: "AGENTS.md", exact: true })).toHaveCount(1); + await panel.getByRole("button", { name: "Open a new tab", exact: true }).click(); + await page.getByRole("option", { name: "Artifacts", exact: true }).click(); + const review = panel.getByRole("button", { name: /Delivered AGENTS.md.*For review/ }); + await review.click(); + await expect(review).toHaveAttribute("aria-expanded", "true"); + await expect(panel.getByRole("heading", { name: "File charter", exact: true })).toBeVisible(); + await expect(review).toContainText("Rev 1"); + await expect(panel.getByRole("link", { name: "Download Delivered AGENTS.md", exact: true })).toBeVisible(); + const cardDownload = page.waitForEvent("download"); + await panel.getByRole("link", { name: "Download: Delivered notes.txt", exact: true }).click(); + expect(await fs.readFile((await (await cardDownload).path())!)).toEqual(files[1]!.buffer); + await expect(panel.getByRole("tab", { name: "Artifacts", exact: true })).toHaveAttribute("aria-selected", "true"); + await panel.getByRole("button", { name: "Open in tab: Delivered notes.txt", exact: true }).click(); + await expect(panel.getByRole("tab", { name: "notes.txt", exact: true })).toHaveAttribute("aria-selected", "true"); + await panel.getByRole("tab", { name: "Artifacts", exact: true }).click(); + await panel.getByRole("button", { name: "Open in tab: Delivered AGENTS.md", exact: true }).click(); + await expect(panel.getByRole("tab", { name: "AGENTS.md", exact: true })).toHaveAttribute("aria-selected", "true"); }); } diff --git a/ui/src/components/issue-properties/IssuePropertiesArtifactsTab.tsx b/ui/src/components/issue-properties/IssuePropertiesArtifactsTab.tsx index ea66b7be3af..11fb1ff498e 100644 --- a/ui/src/components/issue-properties/IssuePropertiesArtifactsTab.tsx +++ b/ui/src/components/issue-properties/IssuePropertiesArtifactsTab.tsx @@ -1,6 +1,5 @@ import { TextAttachmentContext } from "@/context/TextAttachmentContext"; import { isTextAttachment } from "@/lib/issue-attachments"; -import { getAttachmentArtifactWorkProductMetadata } from "@paperclipai/shared"; import { useContext, useEffect, useMemo, useRef, useState } from "react"; import type { CSSProperties } from "react"; import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; @@ -102,6 +101,7 @@ function MarkdownWorkProductRow({ reviewDoc: IssueDocument | undefined; openRequestId?: number; }) { + const openTextAttachment = useContext(TextAttachmentContext); const [expanded, setExpanded] = useState(false); const [annotationPanelOpen, setAnnotationPanelOpen] = useState(false); const headerRef = useRef(null); @@ -236,6 +236,17 @@ function MarkdownWorkProductRow({ onToggle={() => setAnnotationPanelOpen((open) => !open)} /> ) : null} + {openTextAttachment ? ( + + ) : null} { if (row.kind === "work_product") { const wp = row.value; - const textMetadata = getAttachmentArtifactWorkProductMetadata(wp); - if (openTextAttachment && textMetadata && isTextAttachment(textMetadata)) { - return
  • - -
    • ; - } const markdownMetadata = getMarkdownWorkProductAttachmentMetadata(wp); if (markdownMetadata) { const reviewKey = artifactReviewDocumentKey(wp.id); diff --git a/ui/src/components/issue-properties/IssuePropertiesMarkdownWorkProduct.test.tsx b/ui/src/components/issue-properties/IssuePropertiesMarkdownWorkProduct.test.tsx index 2f8f814a4e2..35f7f37844a 100644 --- a/ui/src/components/issue-properties/IssuePropertiesMarkdownWorkProduct.test.tsx +++ b/ui/src/components/issue-properties/IssuePropertiesMarkdownWorkProduct.test.tsx @@ -8,6 +8,8 @@ import type { Issue, IssueAttachment, IssueDocument, IssueWorkProduct } from "@p import { artifactReviewDocumentKey } from "@paperclipai/shared"; import { IssuePropertiesArtifactsTab } from "./IssuePropertiesArtifactsTab"; import { ApiError } from "@/api/client"; +import { TextAttachmentContext } from "@/context/TextAttachmentContext"; +import { RichWorkProductCard } from "@/components/task-chat/RichWorkProductCard"; const mockIssuesApi = vi.hoisted(() => ({ listAttachments: vi.fn(async (): Promise => []), @@ -159,6 +161,7 @@ async function waitForAssertion(assertion: () => void, attempts = 20) { describe("markdown work product review row", () => { let container: HTMLDivElement; let root: Root | null = null; + const openText = vi.fn(); beforeEach(() => { vi.clearAllMocks(); @@ -193,7 +196,9 @@ describe("markdown work product review row", () => { await act(async () => currentRoot.render( - + + + , ), ); @@ -234,6 +239,40 @@ describe("markdown work product review row", () => { expect(mockIssuesApi.ensureWorkProductReviewDocument).not.toHaveBeenCalled(); }); + it("opens the attachment in a tab without replacing its review controls", async () => { + mockUseIssueDocuments.mockReturnValue({ data: [makeReviewDocument({ latestRevisionNumber: 3 })] }); + await renderTab(); + + expect(container.textContent).toContain("Rev 3"); + expect(container.querySelector(`[data-testid="annotation-count-${REVIEW_KEY}"]`)).not.toBeNull(); + const open = container.querySelector('button[aria-label="Open in tab: Verification report"]') as HTMLButtonElement; + expect(open).not.toBeNull(); + await act(async () => open.click()); + expect(openText).toHaveBeenCalledWith(ATTACHMENT_ID, "report.md"); + expect(expandButton().getAttribute("aria-expanded")).toBe("false"); + await act(async () => expandButton().click()); + expect(container.querySelector(`[data-testid="annotation-surface-${REVIEW_KEY}"]`)).not.toBeNull(); + }); + + it.each(["compact", "card"] as const)("keeps %s text-card downloads separate from opening a tab", async (variant) => { + root = createRoot(container); + await act(async () => root!.render( + + + , + )); + const download = container.querySelector('a[aria-label="Download: Verification report"]') as HTMLAnchorElement; + expect(download.getAttribute("href")).toBe(`/api/attachments/${ATTACHMENT_ID}/content?download=1`); + const click = new MouseEvent("click", { bubbles: true, cancelable: true }); + await act(async () => { download.dispatchEvent(click); }); + expect(click.defaultPrevented).toBe(false); + expect(openText).not.toHaveBeenCalled(); + const open = container.querySelector('button[aria-label="Open in tab: Verification report"]') as HTMLButtonElement; + expect(open).not.toBeNull(); + await act(async () => open.click()); + expect(openText).toHaveBeenCalledWith(ATTACHMENT_ID, "report.md"); + }); + it("materializes the document on first expand", async () => { mockIssuesApi.ensureWorkProductReviewDocument.mockResolvedValue(makeReviewDocument()); await renderTab(); diff --git a/ui/src/components/task-chat/RichWorkProductCard.tsx b/ui/src/components/task-chat/RichWorkProductCard.tsx index ae053089389..46581ded661 100644 --- a/ui/src/components/task-chat/RichWorkProductCard.tsx +++ b/ui/src/components/task-chat/RichWorkProductCard.tsx @@ -190,6 +190,7 @@ export function RichWorkProductCard({ workProduct, href, variant = "card" }: Ric const openText = useContext(TextAttachmentContext); const textMetadata = getAttachmentArtifactWorkProductMetadata(workProduct); + const canOpenText = Boolean(openText && textMetadata && isTextAttachment(textMetadata)); const additions = numberMeta(metadata, "additions"); const deletions = numberMeta(metadata, "deletions"); const files = numberMeta(metadata, "files", "changedFiles"); @@ -277,12 +278,22 @@ export function RichWorkProductCard({ workProduct, href, variant = "card" }: Ric
    {chip ? : null} + {canOpenText ? ( + + ) : null} {mediaPath ? ( ) : actionHref ? ( - { if (openText && textMetadata && isTextAttachment(textMetadata) && !event.metaKey && !event.ctrlKey && !event.shiftKey && !event.altKey) { event.preventDefault(); openText(textMetadata.attachmentId, textMetadata.originalFilename ?? workProduct.title); } }} aria-label={`${action}: ${workProduct.title}`} className="inline-flex items-center gap-1 text-xs font-medium text-foreground after:absolute after:inset-0 after:rounded-md focus-visible:outline-none focus-visible:after:ring-2 focus-visible:after:ring-ring" target={actionHref.startsWith("http") ? "_blank" : undefined} rel={actionHref.startsWith("http") ? "noreferrer" : undefined}> + {compact ? null : {action}} ) : null}