mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-24 18:19:05 +02:00
fix(ui): keep artifact tab arrivals passive
Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
e4317c8683
commit
23fc19ffb7
8 files changed
+110
-46
No files matched your search
@@ -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
|
||||
|
||||
|
||||
@@ -0,0 +1,56 @@
|
||||
import { randomUUID } from "node:crypto";
|
||||
import { expect, test, type APIRequestContext } from "@playwright/test";
|
||||
|
||||
async function json(response: Awaited<ReturnType<APIRequestContext["get"]>>) {
|
||||
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<void>((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 });
|
||||
});
|
||||
}
|
||||
@@ -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<HTMLButtonElement>("#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<HTMLButtonElement>("#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<HTMLButtonElement>("#side-panel-tab-artifacts")?.click());
|
||||
await act(async () => container.querySelector<HTMLButtonElement>('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,24 @@ 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("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 () => {
|
||||
|
||||
@@ -390,11 +390,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, {
|
||||
|
||||
@@ -48,12 +48,12 @@ describe("new task artifacts", () => {
|
||||
await act(async () => root.render(<Watch {...props} />));
|
||||
}
|
||||
|
||||
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] });
|
||||
|
||||
@@ -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<string>;
|
||||
ids: Set<string>;
|
||||
}>({ 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.
|
||||
|
||||
@@ -2162,8 +2162,9 @@ describe("IssueDetail", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it.each([false, true])("reveals new artifacts once in the task panel (mobile: %s)", async (isMobile) => {
|
||||
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 +2182,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());
|
||||
|
||||
@@ -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) {
|
||||
|
||||
Reference in new issue
Block a user