diff --git a/doc/AGENT-ARTIFACTS.md b/doc/AGENT-ARTIFACTS.md index a07ee857fa..bc2bcd6fc4 100644 --- a/doc/AGENT-ARTIFACTS.md +++ b/doc/AGENT-ARTIFACTS.md @@ -47,13 +47,14 @@ The command prints issue-safe markdown links for the final task comment. ## Task artifact presentation -While a task is open, a new agent attachment, work product, or document opens -the task's Artifacts tab and reveals the side panel (or mobile drawer). This -uses stored object IDs, so it works with either runner. Uploading a file and -registering its work product counts as one arrival. Existing history, revisions, -and repeated query refreshes preserve the user's tab selection. Plans retain -their existing Plan-tab behavior; unregistered user input attachments remain in -the conversation. +Existing and newly arriving agent attachments, work products, and documents add +the task's Artifacts tab without selecting it, opening the side panel or mobile +drawer, or changing the current document/file link. If the pane is closed, the +tab is available when the user opens it. This uses stored object IDs, so it works +with either runner. Uploading a file and registering its work product counts as +one arrival. Revisions and repeated query refreshes preserve dismissed tabs and +the user's selection. Plans retain their existing Plan-tab behavior; +unregistered user input attachments remain in the conversation. ## Uploaded Artifacts vs Workspace Files diff --git a/tests/e2e/artifact-tab-arrival.spec.ts b/tests/e2e/artifact-tab-arrival.spec.ts new file mode 100644 index 0000000000..a04a54f9b3 --- /dev/null +++ b/tests/e2e/artifact-tab-arrival.spec.ts @@ -0,0 +1,56 @@ +import { randomUUID } from "node:crypto"; +import { expect, test, type APIRequestContext } from "@playwright/test"; + +async function json(response: Awaited>) { + expect(response.ok(), `${response.status()}: ${await response.text()}`).toBe(true); + return response.json(); +} + +// Real document writes, activity delivery, query refresh, storage, and task UI. +// Board-owned tasks avoid invoking an agent merely to test presentation. +for (const mobile of [false, true]) { + test(`artifact arrival preserves a closed panel and composer focus (mobile=${mobile})`, async ({ page, request }, testInfo) => { + await page.setViewportSize(mobile ? { width: 390, height: 844 } : { width: 1440, height: 1000 }); + const company = await json(await request.post("/api/companies", { + data: { name: `Passive artifacts ${randomUUID()}` }, + })); + const issue = await json(await request.post(`/api/companies/${company.id}/issues`, { + data: { title: "Keep reading while an artifact arrives", status: "backlog" }, + })); + await page.addInitScript(() => localStorage.setItem("paperclip:panel-visible", "false")); + await page.goto(`/${company.issuePrefix}/issues/${issue.identifier}?from=inbox&fromHref=%2Finbox%2Fmine`); + await expect(page.getByRole("heading", { name: issue.title, exact: true })).toBeVisible(); + const editor = page.getByTestId("task-chat-composer-input").getByRole("textbox", { name: "editable markdown" }); + await editor.fill("Keep my draft and focus"); + const updated = page.waitForResponse(async (response) => { + if (response.request().method() !== "GET" || !/\/api\/issues\/[^/]+$/.test(new URL(response.url()).pathname)) return false; + const data = await response.json().catch(() => null); + return data?.id === issue.id && data.documentSummaries?.some((doc: { key: string }) => doc.key === "report"); + }); + await json(await request.put(`/api/issues/${issue.id}/documents/report`, { + data: { title: "Arriving report", body: "# Arriving report\n\nDurable output.", format: "markdown" }, + })); + await updated; + // The response has reached the real query client; wait for React to commit. + await page.evaluate(() => new Promise((resolve) => requestAnimationFrame(() => requestAnimationFrame(() => resolve())))); + await expect(editor).toBeFocused(); + await expect(editor).toContainText("Keep my draft and focus"); + await expect(page.getByTestId("mobile-task-side-panel")).toBeHidden(); + expect(await page.evaluate(() => localStorage.getItem("paperclip:panel-visible"))).toBe("false"); + await page.screenshot({ path: testInfo.outputPath("artifact-arrived-panel-closed.png"), fullPage: true }); + + if (mobile) { + await page.getByRole("button", { name: "More actions", exact: true }).click(); + await page.getByRole("button", { name: "Properties", exact: true }).click(); + } else { + await page.getByRole("button", { name: "Show properties", exact: true }).click(); + } + const panel = mobile ? page.getByTestId("mobile-task-side-panel") : page.locator("aside").filter({ has: page.getByRole("tab", { name: "Artifacts", exact: true }) }); + const artifacts = panel.getByRole("tab", { name: "Artifacts", exact: true }); + await expect(artifacts).toBeVisible(); + await expect(artifacts).toHaveAttribute("aria-selected", "false"); + await artifacts.click(); + await expect(panel.getByText("Arriving report", { exact: true })).toBeVisible(); + await page.screenshot({ path: testInfo.outputPath("artifact-opened-by-user.png"), fullPage: true }); + }); +} diff --git a/ui/src/components/task-side-panel/TaskSidePanel.test.tsx b/ui/src/components/task-side-panel/TaskSidePanel.test.tsx index 0436981e2b..c40a81e057 100644 --- a/ui/src/components/task-side-panel/TaskSidePanel.test.tsx +++ b/ui/src/components/task-side-panel/TaskSidePanel.test.tsx @@ -164,11 +164,11 @@ describe("TaskSidePanel", () => { expect(container.textContent).toContain("Properties content"); }); - it("opens Artifacts on a new arrival, preserving manual selection until the next arrival", async () => { + it("adds Artifacts on arrival while preserving the selected tab", async () => { await render(panel()); await act(async () => container.querySelector("#side-panel-tab-properties")?.click()); await render(panel({ artifactsOpenRequestId: 1 })); - expect(container.querySelector('[role="tab"][aria-selected="true"]')?.textContent).toContain("Artifacts"); + expect(container.querySelector('[role="tab"][aria-selected="true"]')?.textContent).toContain("Properties"); await act(async () => container.querySelector("#side-panel-tab-properties")?.click()); fixture.documents = [issueDocument("report", "Updated report")]; @@ -176,17 +176,18 @@ describe("TaskSidePanel", () => { expect(container.querySelector('[role="tab"][aria-selected="true"]')?.textContent).toContain("Properties"); await render(panel({ artifactsOpenRequestId: 2 })); - expect(container.querySelector('[role="tab"][aria-selected="true"]')?.textContent).toContain("Artifacts"); + expect(container.querySelector('[role="tab"][aria-selected="true"]')?.textContent).toContain("Properties"); expect(container.querySelectorAll('[data-side-panel-tab-target="artifacts"]')).toHaveLength(1); }); it("reopens a dismissed Artifacts tab only for a new arrival", async () => { await render(panel({ artifactsOpenRequestId: 1 })); + await act(async () => container.querySelector("#side-panel-tab-artifacts")?.click()); await act(async () => container.querySelector('button[aria-label="Close Artifacts"]')?.click()); await render(panel({ artifactsOpenRequestId: 1 })); expect(container.querySelector('[data-side-panel-tab-target="artifacts"]')).toBeNull(); await render(panel({ artifactsOpenRequestId: 2 })); - expect(container.querySelector('[role="tab"][aria-selected="true"]')?.textContent).toContain("Artifacts"); + expect(container.querySelector('[role="tab"][aria-selected="true"]')?.textContent).toContain("Properties"); }); it("acknowledges an arrival so remounting the panel preserves a later manual selection", async () => { @@ -201,15 +202,46 @@ describe("TaskSidePanel", () => { expect(onArtifactsOpened).toHaveBeenCalledTimes(1); }); - it("clears the workspace-file route when an arriving artifact selects Artifacts", async () => { + it("keeps the workspace-file route and selection when an artifact arrives", async () => { routeFixture.location.search = "?file=ui%2Fsrc%2FApp.tsx&workspace=project"; window.history.replaceState(null, "", `${routeFixture.location.pathname}${routeFixture.location.search}`); await render(panel({ fileTabsEnabled: true })); await render(panel({ fileTabsEnabled: true, artifactsOpenRequestId: 1 })); - expect(container.querySelector('[role="tab"][aria-selected="true"]')?.textContent).toContain("Artifacts"); - expect(routeFixture.navigate).toHaveBeenCalledWith( - expect.objectContaining({ search: "" }), expect.anything(), - ); + expect(container.querySelector('[role="tab"][aria-selected="true"]')?.textContent).toContain("App.tsx"); + expect(container.querySelectorAll('[data-side-panel-tab-target="artifacts"]')).toHaveLength(1); + expect(routeFixture.navigate).not.toHaveBeenCalled(); + }); + + it("preserves an explicitly linked document when artifacts arrive", async () => { + fixture.documents = [issueDocument("agents", "AGENTS.md")]; + const documentDeepLink = { requestId: 1, documentKey: "agents" }; + await render(panel({ documentDeepLink, artifactsOpenRequestId: 1 })); + expect(container.querySelector('[role="tab"][aria-selected="true"]')?.textContent).toContain("AGENTS.md"); + expect(container.querySelectorAll('[data-side-panel-tab-target="artifacts"]')).toHaveLength(1); + }); + + it("handles each document link once without overriding later manual selection", async () => { + fixture.documents = [issueDocument("agents", "AGENTS.md")]; + const documentDeepLink = { requestId: 1, documentKey: "agents" }; + await render(panel({ documentDeepLink })); + await act(async () => container.querySelector("#side-panel-tab-properties")?.click()); + fixture.documents = [...fixture.documents, issueDocument("skill", "SKILL.md")]; + await render(panel({ documentDeepLink: { ...documentDeepLink }, artifactsOpenRequestId: 1 })); + expect(container.querySelector('[role="tab"][aria-selected="true"]')?.textContent).toContain("Properties"); + expect(container.querySelectorAll('[data-side-panel-tab-target="artifacts"]')).toHaveLength(1); + + await render(panel({ documentDeepLink: { ...documentDeepLink, requestId: 2 } })); + expect(container.querySelector('[role="tab"][aria-selected="true"]')?.textContent).toContain("AGENTS.md"); + }); + + it("keeps a document selected when an artifact arrives", async () => { + fixture.documents = [issueDocument("report", "Report")]; + const documentDeepLink = { documentKey: "report", requestId: 1 }; + await render(panel({ documentDeepLink })); + await render(panel({ documentDeepLink, artifactsOpenRequestId: 1 })); + expect(container.querySelector('[role="tab"][aria-selected="true"]')?.textContent).toContain("Report"); + expect(container.querySelectorAll('[data-side-panel-tab-target="artifacts"]')).toHaveLength(1); + expect(container.textContent).toContain("Document report"); }); it("uses the approved pre-rebase tab appearance for Streamlined UI", async () => { diff --git a/ui/src/components/task-side-panel/TaskSidePanel.tsx b/ui/src/components/task-side-panel/TaskSidePanel.tsx index a9e9617f8a..b015561a93 100644 --- a/ui/src/components/task-side-panel/TaskSidePanel.tsx +++ b/ui/src/components/task-side-panel/TaskSidePanel.tsx @@ -262,6 +262,7 @@ export function TaskSidePanel({ const userInteractedRef = useRef(restoredRef.current?.userInteracted ?? false); const autoPlanHandledRef = useRef(restoredRef.current?.autoPlanHandled ?? false); const handledArtifactsRequestRef = useRef(undefined); + const handledDocumentRequestRef = useRef(undefined); const initialState = useMemo(() => { const restored = restoredRef.current?.state; let tabs = restored?.tabs ?? (issue.conversationAgentId ? [taskPanelArtifactsTab()] : [taskPanelPropertiesTab()]); @@ -348,13 +349,15 @@ export function TaskSidePanel({ }, [controller.openTab, planDocument]); useEffect(() => { - if (!documentDeepLink) return; + if (!documentDeepLink || handledDocumentRequestRef.current === documentDeepLink.requestId) return; if ( documentDeepLink.documentKey === "plan" && planDocument === null ) return; const document = documents.find((candidate) => candidate.key === documentDeepLink.documentKey); const label = document ? documentDisplayTitle(document) : documentDeepLink.documentKey === "plan" ? "Plan" : documentDeepLink.documentKey; + // A refresh must not replay a link after the user selects another tab. + handledDocumentRequestRef.current = documentDeepLink.requestId; controller.openTab(taskPanelDocumentTab(documentDeepLink.documentKey, label)); }, [controller.openTab, documentDeepLink, documents, planDocument]); @@ -390,11 +393,11 @@ export function TaskSidePanel({ useEffect(() => { if (artifactsOpenRequestId === undefined || handledArtifactsRequestRef.current === artifactsOpenRequestId) return; handledArtifactsRequestRef.current = artifactsOpenRequestId; - setLauncherOpen(false); - controller.openTab(taskPanelArtifactsTab()); - if (viewer.state || viewer.browse) viewer.close(); + // Background outputs add a discoverable tab without interrupting the + // current document, file, or launcher. Opening the pane is a user action. + controller.openTab(taskPanelArtifactsTab(), false); onArtifactsOpened?.(artifactsOpenRequestId); - }, [artifactsOpenRequestId, controller.openTab, viewer.state, viewer.browse, viewer.close, onArtifactsOpened]); + }, [artifactsOpenRequestId, controller.openTab, onArtifactsOpened]); const recentFilesQuery = useQuery({ queryKey: queryKeys.issues.fileResources(issue.id, { diff --git a/ui/src/hooks/useTaskArtifactArrival.test.tsx b/ui/src/hooks/useTaskArtifactArrival.test.tsx index 00e7a16ff8..6f728bee70 100644 --- a/ui/src/hooks/useTaskArtifactArrival.test.tsx +++ b/ui/src/hooks/useTaskArtifactArrival.test.tsx @@ -48,12 +48,12 @@ describe("new task artifacts", () => { await act(async () => root.render()); } - it("baselines independently loaded history without opening the panel", async () => { + it("registers existing artifacts as their queries load", async () => { await render({ attachments: undefined, workProducts: undefined, documents: undefined }); await render({ attachments: [attachment()] }); await render({ documents: [document()] }); await render({ workProducts: [product()] }); - expect(onArrival).not.toHaveBeenCalled(); + expect(onArrival).toHaveBeenCalledTimes(2); }); it.each(["attachments", "workProducts", "documents"] as const)("reveals new %s after the initial load", async (source) => { @@ -87,13 +87,13 @@ describe("new task artifacts", () => { expect(onArrival).toHaveBeenCalledTimes(2); }); - it("baselines a newly navigated task and detects its subsequent additions", async () => { + it("registers a newly navigated task and detects its subsequent additions", async () => { await render(); await render({ issueId: "task-2", attachments: undefined, documents: undefined, workProducts: undefined }); await render({ attachments: [attachment()], documents: [], workProducts: [] }); - expect(onArrival).not.toHaveBeenCalled(); - await render({ documents: [document()] }); expect(onArrival).toHaveBeenCalledOnce(); + await render({ documents: [document()] }); + expect(onArrival).toHaveBeenCalledTimes(2); }); it("leaves Plan handling and user input uploads alone, but reveals a published user file", async () => { @@ -106,6 +106,7 @@ describe("new task artifacts", () => { it("ignores document revision changes and watches non-file work products", async () => { await render({ documents: [document()] }); + onArrival.mockClear(); await render({ documents: [{ ...document(), latestRevisionNumber: 2 }] }); expect(onArrival).not.toHaveBeenCalled(); await render({ workProducts: [{ id: "pr-1", type: "pull_request", provider: "github" } as IssueWorkProduct] }); diff --git a/ui/src/hooks/useTaskArtifactArrival.ts b/ui/src/hooks/useTaskArtifactArrival.ts index 88299dea08..561cf62a08 100644 --- a/ui/src/hooks/useTaskArtifactArrival.ts +++ b/ui/src/hooks/useTaskArtifactArrival.ts @@ -16,19 +16,18 @@ interface TaskArtifactArrivalOptions { onArrival: () => void; } -/** Watch the same durable objects as the Artifacts tab, excluding its initial load. */ +/** Register the same durable objects as the Artifacts tab, including history. */ export function useTaskArtifactArrival({ issueId, attachments, workProducts, documents, onArrival, }: TaskArtifactArrivalOptions) { const observed = useRef<{ issueId: string | undefined; - loaded: Set; ids: Set; - }>({ issueId: undefined, loaded: new Set(), ids: new Set() }); + }>({ issueId: undefined, ids: new Set() }); useEffect(() => { if (observed.current.issueId !== issueId) { - observed.current = { issueId, loaded: new Set(), ids: new Set() }; + observed.current = { issueId, ids: new Set() }; } if (!issueId) return; @@ -46,14 +45,12 @@ export function useTaskArtifactArrival({ }; const state = observed.current; let arrived = false; - for (const [source, ids] of Object.entries(sources)) { + for (const ids of Object.values(sources)) { if (!ids) continue; - const loaded = state.loaded.has(source); for (const id of ids) { - if (loaded && !state.ids.has(id)) arrived = true; + if (!state.ids.has(id)) arrived = true; state.ids.add(id); } - state.loaded.add(source); } // Keep observed IDs across removals and failed refetches. A repeated // snapshot or a revision update must not take the user's tab selection. diff --git a/ui/src/pages/IssueDetail.test.tsx b/ui/src/pages/IssueDetail.test.tsx index 089ba6a08d..30294009db 100644 --- a/ui/src/pages/IssueDetail.test.tsx +++ b/ui/src/pages/IssueDetail.test.tsx @@ -2162,8 +2162,32 @@ describe("IssueDetail", () => { }); }); - it.each([false, true])("reveals new artifacts once in the task panel (mobile: %s)", async (isMobile) => { + it("opens Artifacts for existing output without discarding a document deep link", async () => { + mockLocation.hash = "#document-agents"; + mockIssuesApi.get.mockResolvedValue(createIssue({ + documentSummaries: [{ + id: "agents-doc", companyId: "company-1", issueId: "issue-1", + key: "agents", title: "AGENTS.md", format: "markdown", + latestRevisionId: "revision-1", latestRevisionNumber: 1, + createdByAgentId: "agent-1", createdByUserId: null, + updatedByAgentId: "agent-1", updatedByUserId: null, + lockedAt: null, lockedByAgentId: null, lockedByUserId: null, + createdAt: new Date(), updatedAt: new Date(), + }], + })); + await act(async () => { + root.render(); + }); + await waitForAssertion(() => { + const props = mockOpenPanel.mock.calls.at(-1)?.[0]?.props.children?.props; + expect(props?.artifactsOpenRequestId).toBe(1); + expect(props?.documentDeepLink?.documentKey).toBe("agents"); + }); + }); + + it.each([false, true])("registers new artifacts without opening a closed panel (mobile: %s)", async (isMobile) => { mockSidebarState.isMobile = isMobile; + mockLocation.state = createIssueDetailLocationState("Inbox", "/inbox/mine", "inbox"); mockPanelState.panelVisible = false; mockIssuesApi.get.mockResolvedValue(createIssue()); await act(async () => { @@ -2181,14 +2205,15 @@ describe("IssueDetail", () => { }; const file = createAttachment({ id: "new-output", createdByAgentId: "agent-1" }); act(() => { queryClient.setQueryData(queryKeys.issues.attachments("PAP-1"), [file]); }); - await waitForAssertion(() => expect(panelProps()?.artifactsOpenRequestId).toBe(1)); + await flushReact(); + expect(mockSetPanelVisible).not.toHaveBeenCalled(); + expect(document.querySelector('[data-testid="mobile-task-side-panel"]')).toBeNull(); if (isMobile) { - expect(document.querySelector('[data-testid="mobile-task-side-panel"]')).not.toBeNull(); - expect(mockSetPanelVisible).not.toHaveBeenCalled(); - expect(mockOpenPanel.mock.calls.at(-1)?.[0]?.props.children?.props.artifactsOpenRequestId).toBeUndefined(); - } else { - expect(mockSetPanelVisible).toHaveBeenCalledWith(true); + const toolbar = mockSetMobileToolbar.mock.calls.map(([node]) => node).filter(Boolean).at(-1); + // The pending arrival is consumed only when the user opens the sheet. + act(() => toolbar.props.onProperties()); } + await waitForAssertion(() => expect(panelProps()?.artifactsOpenRequestId).toBe(1)); act(() => panelProps().onArtifactsOpened(1)); await waitForAssertion(() => expect(panelProps().artifactsOpenRequestId).toBeUndefined()); diff --git a/ui/src/pages/IssueDetail.tsx b/ui/src/pages/IssueDetail.tsx index da2a2b4df5..02a39a0b6e 100644 --- a/ui/src/pages/IssueDetail.tsx +++ b/ui/src/pages/IssueDetail.tsx @@ -3606,16 +3606,13 @@ export function TaskDetailSurface({ conversation, tasksTab }: { tasksTab?: TaskS setPanelVisible(true); if (isMobile) setMobilePropsOpen(true); }, [isMobile, issue?.id, panelBeforePlanOverrideIssueId, setPanelVisible, suppressPanelUntilPlan]); - const revealNewArtifact = useCallback(() => { + const registerArtifactTab = useCallback(() => { if (!issue?.id) return; - setDocumentDeepLink(null); setArtifactsOpenRequest((previous) => ({ issueId: issue.id, requestId: (previous?.requestId ?? 0) + 1, })); - if (isMobile) setMobilePropsOpen(true); - else openTaskSidePanel(); - }, [issue?.id, isMobile, openTaskSidePanel]); + }, [issue?.id]); const handleArtifactsOpened = useCallback((requestId: number) => { setArtifactsOpenRequest((request) => request?.requestId === requestId ? { ...request, handled: true } : request); @@ -3625,7 +3622,7 @@ export function TaskDetailSurface({ conversation, tasksTab }: { tasksTab?: TaskS attachments, workProducts, documents: issue?.documentSummaries, - onArrival: revealNewArtifact, + onArrival: registerArtifactTab, }); const toggleTaskSidePanel = useCallback(() => { if (!panelVisible || suppressPanelUntilPlan) {