From 4297f192f9181aa1ba35413f1a0004ad220638d0 Mon Sep 17 00:00:00 2001 From: Shy Alter Date: Tue, 28 Jul 2026 12:39:09 +0200 Subject: [PATCH 1/7] feat(channels): add the structured Activity view Replace the flag-gated task thread with Timeline, Artifacts, and Comments views. Include multi-run artifacts and selected historical pull request review while leaving flag-off behavior unchanged. Generated-By: PostHog Code Task-Id: 6190e713-9b80-43d9-a05c-5e3b1ecdf297 --- .../src/canvas/runArtifactSchemas.test.ts | 36 ++ .../core/src/canvas/runArtifactSchemas.ts | 17 + .../canvas/components/ActivityPanel.tsx | 265 ++++++++++++++ .../canvas/components/ActivityTimeline.tsx | 262 ++++++++++++++ .../components/TaskArtifactsList.test.tsx | 63 ++++ .../canvas/components/TaskArtifactsList.tsx | 328 ++++++++++++++++++ .../canvas/components/ThreadSidebar.tsx | 16 +- .../src/features/canvas/hooks/useTaskRuns.ts | 20 ++ .../components/CloudReviewPage.tsx | 5 +- .../code-review/components/ReviewPage.tsx | 5 +- .../hooks/useEffectiveDiffSource.ts | 10 +- .../code-review/reviewNavigationStore.test.ts | 17 + .../code-review/reviewNavigationStore.ts | 15 + .../task-detail/hooks/useCloudChangedFiles.ts | 5 +- 14 files changed, 1051 insertions(+), 13 deletions(-) create mode 100644 packages/core/src/canvas/runArtifactSchemas.test.ts create mode 100644 packages/core/src/canvas/runArtifactSchemas.ts create mode 100644 packages/ui/src/features/canvas/components/ActivityPanel.tsx create mode 100644 packages/ui/src/features/canvas/components/ActivityTimeline.tsx create mode 100644 packages/ui/src/features/canvas/components/TaskArtifactsList.test.tsx create mode 100644 packages/ui/src/features/canvas/components/TaskArtifactsList.tsx create mode 100644 packages/ui/src/features/canvas/hooks/useTaskRuns.ts diff --git a/packages/core/src/canvas/runArtifactSchemas.test.ts b/packages/core/src/canvas/runArtifactSchemas.test.ts new file mode 100644 index 0000000000..e2db2fe283 --- /dev/null +++ b/packages/core/src/canvas/runArtifactSchemas.test.ts @@ -0,0 +1,36 @@ +import { describe, expect, it } from "vitest"; +import { parseRunPlans } from "./runArtifactSchemas"; + +describe("parseRunPlans", () => { + it.each([ + { name: "undefined", raw: undefined }, + { name: "null", raw: null }, + { name: "an object", raw: { artifacts: [] } }, + { name: "a string", raw: "plan" }, + ])("reads $name as no artifacts", ({ raw }) => { + expect(parseRunPlans(raw)).toEqual([]); + }); + + it("keeps only plan artifacts", () => { + const plans = parseRunPlans([ + { id: "a", type: "plan", name: "Plan A" }, + { id: "b", type: "upload", name: "internal blob" }, + ]); + expect(plans).toEqual([{ id: "a", type: "plan", name: "Plan A" }]); + }); + + // One bad entry shouldn't take the Artifacts tab down with it. + it("drops entries that don't match the shape", () => { + const plans = parseRunPlans([ + { id: 42, type: "plan" }, + null, + "not an object", + { type: "plan", storage_path: "runs/1/plan.md" }, + ]); + expect(plans).toEqual([{ type: "plan", storage_path: "runs/1/plan.md" }]); + }); + + it("ignores an artifact with no type", () => { + expect(parseRunPlans([{ id: "a", name: "mystery" }])).toEqual([]); + }); +}); diff --git a/packages/core/src/canvas/runArtifactSchemas.ts b/packages/core/src/canvas/runArtifactSchemas.ts new file mode 100644 index 0000000000..8d95e86e32 --- /dev/null +++ b/packages/core/src/canvas/runArtifactSchemas.ts @@ -0,0 +1,17 @@ +import { z } from "zod"; + +export const runArtifactSchema = z.object({ + id: z.string().optional(), + name: z.string().optional(), + type: z.string().optional(), + storage_path: z.string().optional(), +}); +export type RunArtifact = z.infer; + +export function parseRunPlans(raw: unknown): RunArtifact[] { + if (!Array.isArray(raw)) return []; + return raw.flatMap((entry) => { + const parsed = runArtifactSchema.safeParse(entry); + return parsed.success && parsed.data.type === "plan" ? [parsed.data] : []; + }); +} diff --git a/packages/ui/src/features/canvas/components/ActivityPanel.tsx b/packages/ui/src/features/canvas/components/ActivityPanel.tsx new file mode 100644 index 0000000000..06d075f578 --- /dev/null +++ b/packages/ui/src/features/canvas/components/ActivityPanel.tsx @@ -0,0 +1,265 @@ +import { CaretRightIcon } from "@phosphor-icons/react"; +import { Button, cn, Tabs, TabsList, TabsTrigger } from "@posthog/quill"; +import { ANALYTICS_EVENTS } from "@posthog/shared/analytics-events"; +import type { Task } from "@posthog/shared/domain-types"; +import { ActivityTimeline } from "@posthog/ui/features/canvas/components/ActivityTimeline"; +import { TaskCard } from "@posthog/ui/features/canvas/components/ChannelFeedView"; +import { TaskArtifactsList } from "@posthog/ui/features/canvas/components/TaskArtifactsList"; +import { + AgentStatusLine, + ThreadLoadingState, + ThreadPanelHeader, + ThreadReplyComposer, + ThreadTimeline, +} from "@posthog/ui/features/canvas/components/ThreadPanel"; +import { useThreadConversation } from "@posthog/ui/features/canvas/hooks/useThreadConversation"; +import { buildConversationItems } from "@posthog/ui/features/sessions/components/buildConversationItems"; +import { taskDetailQuery } from "@posthog/ui/features/tasks/queries"; +import { track } from "@posthog/ui/shell/analytics"; +import { useQuery } from "@tanstack/react-query"; +import { useCallback, useEffect, useMemo, useRef, useState } from "react"; + +type ActivityTab = "timeline" | "artifacts" | "comments"; + +const ACTIVITY_TABS: readonly { key: ActivityTab; label: string }[] = [ + { key: "timeline", label: "Timeline" }, + { key: "artifacts", label: "Artifacts" }, + { key: "comments", label: "Comments" }, +] as const; + +const TABS_WITH_COMPOSER: ReadonlySet = new Set([ + "timeline", + "comments", +]); + +const TIMESTAMP_END_CLASS = + "[&_[data-slot=thread-item-timestamp]]:ml-auto [&_[data-slot=thread-item-timestamp]]:shrink-0 [&_[data-slot=thread-item-timestamp]]:pl-2"; + +function ActivityTabsRow({ + tab, + onTabChange, +}: { + tab: ActivityTab; + onTabChange: (tab: ActivityTab) => void; +}) { + return ( +
+ onTabChange(value as ActivityTab)} + > + + {ACTIVITY_TABS.map((t) => ( + + {t.label} + + ))} + + +
+ ); +} + +function ActivityConversation({ + task, + channelId, + onClose, + onToggleCollapsed, + onOpenFull, + showTaskSummary, +}: { + task: Task; + channelId: string; + onClose?: () => void; + onToggleCollapsed?: () => void; + onOpenFull?: () => void; + showTaskSummary: boolean; +}) { + const taskId = task.id; + const { + timeline, + agentStatus, + events, + isPromptPending, + isReady, + members, + currentUser, + isTaskAuthor, + canForward, + draft, + setDraft, + isSubmitDisabled, + submit, + sendMessageToAgent, + deleteMessage, + onMentionInsert, + } = useThreadConversation(task, { surface: "activity_panel" }); + + const [tab, setTab] = useState("timeline"); + const handleTabChange = useCallback( + (next: ActivityTab) => { + setTab(next); + track(ANALYTICS_EVENTS.CHANNEL_ACTION, { + action_type: "activity_tab_change", + surface: "activity_panel", + task_id: taskId, + tab: next, + }); + }, + [taskId], + ); + + const commentRows = useMemo( + () => timeline.filter((row) => row.kind === "human"), + [timeline], + ); + const conversationItems = useMemo( + () => + tab === "timeline" + ? buildConversationItems(events, isPromptPending).items + : [], + [tab, events, isPromptPending], + ); + + const scrollRef = useRef(null); + // biome-ignore lint/correctness/useExhaustiveDependencies: scroll when rendered thread content changes + useEffect(() => { + scrollRef.current?.scrollTo({ top: scrollRef.current.scrollHeight }); + }, [timeline, events.length, agentStatus?.phase, tab]); + + const showComposer = TABS_WITH_COMPOSER.has(tab); + + const body = () => { + if (tab === "artifacts") { + return ; + } + if (tab === "comments") { + return ( + + ); + } + if (!isReady) return ; + return ( + + ); + }; + + return ( +
+ + + + {showTaskSummary && ( +
+ +
+ )} +
+ {body()} +
+ + {showComposer && agentStatus && } + + {showComposer && ( + + )} +
+ ); +} + +export function ActivityPanel({ + taskId, + channelId, + task: taskProp, + onClose, + collapsed, + onToggleCollapsed, + onOpenFull, + showTaskSummary = true, +}: { + taskId: string; + channelId: string; + task?: Task; + onClose?: () => void; + collapsed?: boolean; + onToggleCollapsed?: () => void; + onOpenFull?: () => void; + showTaskSummary?: boolean; +}) { + const { data: fetchedTask } = useQuery({ + ...taskDetailQuery(taskId), + enabled: !taskProp && !collapsed, + }); + const task = taskProp ?? fetchedTask; + + if (collapsed) { + return ( +
+ +
+ ); + } + + if (!task) { + return ; + } + + return ( + + ); +} diff --git a/packages/ui/src/features/canvas/components/ActivityTimeline.tsx b/packages/ui/src/features/canvas/components/ActivityTimeline.tsx new file mode 100644 index 0000000000..482aa35c50 --- /dev/null +++ b/packages/ui/src/features/canvas/components/ActivityTimeline.tsx @@ -0,0 +1,262 @@ +import { CheckCircleIcon, XCircleIcon } from "@phosphor-icons/react"; +import type { ThreadTimelineRow } from "@posthog/core/canvas/threadTimeline"; +import { + ThreadItem, + ThreadItemAuthor, + ThreadItemBody, + ThreadItemContent, + ThreadItemGroup, + ThreadItemGutter, + ThreadItemHeader, +} from "@posthog/quill"; +import type { + Task, + TaskThreadMessage, + UserBasic, +} from "@posthog/shared/domain-types"; +import { isTerminalStatus } from "@posthog/shared/domain-types"; +import { UserAvatar } from "@posthog/ui/features/auth/UserAvatar"; +import { + ThreadArtifactRow, + ThreadMessageRow, +} from "@posthog/ui/features/canvas/components/ThreadPanel"; +import { ThreadTimestamp } from "@posthog/ui/features/canvas/components/ThreadTimestamp"; +import { userDisplayName } from "@posthog/ui/features/canvas/utils/userDisplay"; +import type { buildConversationItems } from "@posthog/ui/features/sessions/components/buildConversationItems"; +import { Fragment, type ReactNode, useMemo } from "react"; + +type ConversationItem = ReturnType< + typeof buildConversationItems +>["items"][number]; + +function ActivityEventRow({ + node, + title, + action, + timestamp, +}: { + node: ReactNode; + title: string; + action?: string; + timestamp: string; +}) { + return ( +
+
+
{node}
+
+ + {title} + {action && {action}} + + +
+ ); +} + +function EventNode({ icon }: { icon: ReactNode }) { + return ( + + {icon} + + ); +} + +function UserMessageRow({ + author, + content, + timestamp, +}: { + author?: UserBasic | null; + content: string; + timestamp: string; +}) { + return ( + + + + + + + + {author ? userDisplayName(author) : "You"} + + + + + + {content} + + + + + ); +} + +export function ActivityTimeline({ + task, + timeline, + conversationItems, + currentUserUuid, + currentUserEmail, + isTaskAuthor, + canForward, + onSendToAgent, + onDelete, +}: { + task: Task; + timeline: ThreadTimelineRow[]; + conversationItems: ConversationItem[]; + currentUserUuid?: string; + currentUserEmail?: string | null; + isTaskAuthor: boolean; + canForward: boolean; + onSendToAgent: (messageId: string) => void; + onDelete: (messageId: string) => void; +}) { + const nodes = useMemo(() => { + const entries: { key: string; ts: number; node: ReactNode }[] = []; + const createdTs = Date.parse(task.created_at) || 0; + entries.push({ + key: "task-created", + ts: createdTs, + node: ( + + } + title={task.created_by ? userDisplayName(task.created_by) : "Someone"} + action="created this task" + timestamp={task.created_at} + /> + ), + }); + + for (const item of conversationItems) { + if (item.type !== "user_message") continue; + entries.push({ + key: `user-message-${item.id}`, + ts: item.timestamp, + node: ( + + ), + }); + } + + let hasPrArtifact = false; + for (const row of timeline) { + if (row.kind === "artifact" && row.artifact.kind === "pr") { + hasPrArtifact = true; + } + entries.push({ + key: `thread-${row.message.id}`, + ts: row.timestamp, + node: + row.kind === "human" ? ( + onSendToAgent(row.message.id)} + onDelete={() => onDelete(row.message.id)} + /> + ) : ( + + ), + }); + } + + const updatedTs = Date.parse(task.updated_at) || createdTs; + const outputPr = task.latest_run?.output?.pr_url; + if (typeof outputPr === "string" && outputPr && !hasPrArtifact) { + entries.push({ + key: "output-pr", + ts: updatedTs, + node: ( + + ), + }); + } + + const runStatus = task.latest_run?.status; + if (runStatus && isTerminalStatus(runStatus)) { + const succeeded = runStatus === "completed"; + entries.push({ + key: "run-status", + ts: updatedTs + 1, + node: ( + + ) : ( + + ) + } + /> + } + title={`Task ${runStatus.replace(/_/g, " ")}`} + timestamp={task.updated_at} + /> + ), + }); + } + + return entries.sort((a, b) => a.ts - b.ts); + }, [ + conversationItems, + timeline, + task, + isTaskAuthor, + canForward, + currentUserUuid, + currentUserEmail, + onSendToAgent, + onDelete, + ]); + + return ( +
+
+
+ + {nodes.map((entry) => ( + {entry.node} + ))} + +
+
+ ); +} diff --git a/packages/ui/src/features/canvas/components/TaskArtifactsList.test.tsx b/packages/ui/src/features/canvas/components/TaskArtifactsList.test.tsx new file mode 100644 index 0000000000..42782087fc --- /dev/null +++ b/packages/ui/src/features/canvas/components/TaskArtifactsList.test.tsx @@ -0,0 +1,63 @@ +import type { Task, TaskRun } from "@posthog/shared/domain-types"; +import { fireEvent, render, screen } from "@testing-library/react"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const mocks = vi.hoisted(() => ({ + runs: [] as TaskRun[], +})); + +vi.mock("@posthog/ui/features/canvas/hooks/useTaskRuns", () => ({ + useTaskRuns: () => ({ runs: mocks.runs, isLoading: false }), +})); +vi.mock("@posthog/ui/features/git-interaction/usePrArtifact", () => ({ + usePrArtifact: (url: string) => ({ + safeUrl: url, + title: `Pull request #${url.split("/").at(-1)}`, + stateLabel: "Open", + Icon: () => , + iconColor: "currentColor", + }), +})); +vi.mock("@posthog/ui/features/pr-review/usePrComments", () => ({ + usePrComments: () => ({ data: undefined }), +})); +vi.mock("@posthog/ui/features/pr-review/usePrReviewThreads", () => ({ + usePrReviewThreads: () => ({ data: undefined }), +})); + +import { useReviewNavigationStore } from "@posthog/ui/features/code-review/reviewNavigationStore"; +import { TaskArtifactsList } from "./TaskArtifactsList"; + +const task = { + id: "task-1", + latest_run: null, +} as unknown as Task; + +function run(id: string, prNumber: number): TaskRun { + return { + id, + output: { pr_url: `https://github.com/acme/repo/pull/${prNumber}` }, + } as unknown as TaskRun; +} + +describe("TaskArtifactsList", () => { + beforeEach(() => { + mocks.runs = [run("run-1", 1), run("run-2", 2)]; + useReviewNavigationStore.setState({ + reviewModes: {}, + selectedPrUrls: {}, + }); + }); + + it("opens the PR represented by the selected historical row", () => { + render(); + + fireEvent.click(screen.getByText("Pull request #2")); + + const state = useReviewNavigationStore.getState(); + expect(state.selectedPrUrls[task.id]).toBe( + "https://github.com/acme/repo/pull/2", + ); + expect(state.reviewModes[task.id]).toBe("split"); + }); +}); diff --git a/packages/ui/src/features/canvas/components/TaskArtifactsList.tsx b/packages/ui/src/features/canvas/components/TaskArtifactsList.tsx new file mode 100644 index 0000000000..0f50a76fbe --- /dev/null +++ b/packages/ui/src/features/canvas/components/TaskArtifactsList.tsx @@ -0,0 +1,328 @@ +import { + ArrowSquareOutIcon, + ClipboardTextIcon, + PackageIcon, + SlackLogoIcon, +} from "@phosphor-icons/react"; +import { + parseRunPlans, + type RunArtifact, +} from "@posthog/core/canvas/runArtifactSchemas"; +import type { ThreadTimelineRow } from "@posthog/core/canvas/threadTimeline"; +import { + Empty, + EmptyDescription, + EmptyHeader, + EmptyMedia, + EmptyTitle, +} from "@posthog/quill"; +import type { + Task, + TaskRun, + TaskThreadMessage, +} from "@posthog/shared/domain-types"; +import { useOptionalAuthenticatedClient } from "@posthog/ui/features/auth/authClient"; +import { iconForTemplate } from "@posthog/ui/features/canvas/components/canvasTemplateIcon"; +import { useTaskRuns } from "@posthog/ui/features/canvas/hooks/useTaskRuns"; +import { useReviewNavigationStore } from "@posthog/ui/features/code-review/reviewNavigationStore"; +import { usePrArtifact } from "@posthog/ui/features/git-interaction/usePrArtifact"; +import { usePrComments } from "@posthog/ui/features/pr-review/usePrComments"; +import { usePrReviewThreads } from "@posthog/ui/features/pr-review/usePrReviewThreads"; +import { toast } from "@posthog/ui/primitives/toast"; +import { openExternalUrl } from "@posthog/ui/shell/openExternal"; +import { parseHttpsUrl, parseShareLink } from "@posthog/ui/utils/posthogLinks"; +import { navigateToShareTarget } from "@posthog/ui/utils/shareLinks"; +import { getPostHogUrl } from "@posthog/ui/utils/urls"; +import { type ReactNode, useMemo, useState } from "react"; + +type ArtifactRow = + | { kind: "pr"; key: string; url: string } + | { kind: "canvas"; key: string; name: string; url: string | null } + | { + kind: "plan"; + key: string; + name: string; + storagePath: string | null; + runId: string | null; + } + | { kind: "slack"; key: string; url: string }; + +function readRunPlans(run: TaskRun): RunArtifact[] { + return parseRunPlans((run as { artifacts?: unknown }).artifacts); +} + +const UUID_RE = + /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}/i; +function planTitle(name: string | undefined): string { + if (!name || UUID_RE.test(name)) return "Plan"; + return name; +} + +function buildRows( + task: Task, + timeline: ThreadTimelineRow[], + runs: TaskRun[], +): ArtifactRow[] { + const rows: ArtifactRow[] = []; + const seenPrUrls = new Set(); + const seenPlanKeys = new Set(); + + const addPr = (url: string, key: string) => { + if (seenPrUrls.has(url)) return; + seenPrUrls.add(url); + rows.push({ kind: "pr", key, url }); + }; + + for (const row of timeline) { + if (row.kind !== "artifact") continue; + if (row.artifact.kind === "pr") { + addPr(row.artifact.url, row.message.id); + } else { + rows.push({ + kind: "canvas", + key: row.message.id, + name: row.artifact.name, + url: row.artifact.url, + }); + } + } + + const allRuns = + runs.length > 0 ? runs : task.latest_run ? [task.latest_run] : []; + for (const run of allRuns) { + const outputPr = run.output?.pr_url; + if (typeof outputPr === "string" && outputPr) { + addPr(outputPr, `output-pr:${outputPr}`); + } + for (const plan of readRunPlans(run)) { + const key = `plan:${plan.id ?? plan.storage_path ?? plan.name}`; + if (seenPlanKeys.has(key)) continue; + seenPlanKeys.add(key); + rows.push({ + kind: "plan", + key, + name: planTitle(plan.name), + storagePath: plan.storage_path ?? null, + runId: run.id, + }); + } + } + + const slackUrl = task.latest_run?.state?.slack_thread_url; + if (typeof slackUrl === "string" && slackUrl) { + rows.push({ kind: "slack", key: "slack-thread", url: slackUrl }); + } + + return rows; +} + +function ArtifactListRow({ + icon, + title, + detail, + external, + onOpen, + onHoverStart, +}: { + icon: ReactNode; + title: string; + detail?: string | null; + external?: boolean; + onOpen?: () => void; + onHoverStart?: () => void; +}) { + return ( + + ); +} + +function PrRow({ url, taskId }: { url: string; taskId: string }) { + const { safeUrl, title, stateLabel, Icon, iconColor } = usePrArtifact(url); + const setReviewMode = useReviewNavigationStore((s) => s.setReviewMode); + const setSelectedPrUrl = useReviewNavigationStore((s) => s.setSelectedPrUrl); + + const [countsWanted, setCountsWanted] = useState(false); + const comments = usePrComments(countsWanted ? safeUrl : null); + const threads = usePrReviewThreads(countsWanted ? safeUrl : null); + + const commentCount = + (comments.data?.length ?? 0) + + (threads.data ?? []).reduce( + (sum, thread) => sum + thread.comments.length, + 0, + ); + const detailParts = [ + stateLabel, + comments.data || threads.data + ? `${commentCount} ${commentCount === 1 ? "comment" : "comments"}` + : null, + ].filter(Boolean); + + return ( + + } + title={title} + detail={detailParts.join(" · ") || null} + onHoverStart={() => setCountsWanted(true)} + onOpen={ + safeUrl + ? () => { + setSelectedPrUrl(taskId, safeUrl); + setReviewMode(taskId, "split"); + } + : undefined + } + /> + ); +} + +function CanvasRow({ name, url }: { name: string; url: string | null }) { + const parsed = url ? parseHttpsUrl(url) : null; + const target = parsed ? parseShareLink(parsed.href) : null; + const open = + parsed && target + ? () => { + const currentPostHogUrl = getPostHogUrl("/"); + const currentPostHogOrigin = currentPostHogUrl + ? parseHttpsUrl(currentPostHogUrl)?.origin + : null; + if (parsed.origin === currentPostHogOrigin) { + navigateToShareTarget(target); + } else { + openExternalUrl(parsed.href); + } + } + : undefined; + return ( + + ); +} + +function PlanRow({ + taskId, + runId, + name, + storagePath, +}: { + taskId: string; + runId: string | null; + name: string; + storagePath: string | null; +}) { + const client = useOptionalAuthenticatedClient(); + const canOpen = !!client && !!runId && !!storagePath; + const onOpen = canOpen + ? () => { + client + .presignTaskRunArtifact( + taskId, + runId as string, + storagePath as string, + ) + .then((url) => openExternalUrl(url)) + .catch((error: unknown) => { + toast.error("Couldn't open plan", { + description: + error instanceof Error ? error.message : String(error), + }); + }); + } + : undefined; + return ( + } + title={name} + detail="Plan" + external={canOpen} + onOpen={onOpen} + /> + ); +} + +export function TaskArtifactsList({ + task, + timeline, +}: { + task: Task; + timeline: ThreadTimelineRow[]; +}) { + const { runs } = useTaskRuns(task.id); + const rows = useMemo( + () => buildRows(task, timeline, runs), + [task, timeline, runs], + ); + + if (rows.length === 0) { + return ( + + + + + + No artifacts yet + + Pull requests and canvases produced while working on this task show + up here. + + + + ); + } + + return ( +
+ {rows.map((row) => + row.kind === "pr" ? ( + + ) : row.kind === "canvas" ? ( + + ) : row.kind === "plan" ? ( + + ) : ( + } + title="Slack thread" + detail="External" + external + onOpen={() => openExternalUrl(row.url)} + /> + ), + )} +
+ ); +} diff --git a/packages/ui/src/features/canvas/components/ThreadSidebar.tsx b/packages/ui/src/features/canvas/components/ThreadSidebar.tsx index 4a81cf8700..1dfa13cb29 100644 --- a/packages/ui/src/features/canvas/components/ThreadSidebar.tsx +++ b/packages/ui/src/features/canvas/components/ThreadSidebar.tsx @@ -1,15 +1,15 @@ import { ANALYTICS_EVENTS } from "@posthog/shared/analytics-events"; import type { Task } from "@posthog/shared/domain-types"; +import { ActivityPanel } from "@posthog/ui/features/canvas/components/ActivityPanel"; import { ThreadPanel } from "@posthog/ui/features/canvas/components/ThreadPanel"; +import { useChannelsLayout } from "@posthog/ui/features/canvas/hooks/useChannelsLayout"; import { useThreadPanelStore } from "@posthog/ui/features/canvas/stores/threadPanelStore"; import { ResizableSidebar } from "@posthog/ui/primitives/ResizableSidebar"; import { track } from "@posthog/ui/shell/analytics"; import { useState } from "react"; -// The right-hand dock hosting a task's ThreadPanel: a thin rail when -// collapsed, a resizable sidebar otherwise. Shared by the channel feed and the -// task detail route; owns the panel-store size/collapse reads so parents don't -// re-render on every resize tick. +// The right-hand dock for a task's thread (collapsible, resizable). Flag on +// swaps the legacy ThreadPanel for the tabbed ActivityPanel. export function ThreadSidebar({ taskId, channelId, @@ -31,19 +31,21 @@ export function ThreadSidebar({ const setWidth = useThreadPanelStore((s) => s.setWidth); const setCollapsed = useThreadPanelStore((s) => s.setCollapsed); const [isResizing, setIsResizing] = useState(false); + const channelsLayout = useChannelsLayout(); + const Panel = channelsLayout ? ActivityPanel : ThreadPanel; const toggleCollapsed = (next: boolean) => { setCollapsed(next); track(ANALYTICS_EVENTS.CHANNEL_ACTION, { action_type: next ? "collapse_thread" : "expand_thread", - surface: "thread_panel", + surface: channelsLayout ? "activity_panel" : "thread_panel", task_id: taskId, }); }; if (collapsed) { return ( - - ( + ["task-runs", taskId ?? "none"], + (client) => client.listTaskRuns(taskId as string), + { + enabled: !!taskId, + refetchInterval: TASK_RUNS_POLL_INTERVAL_MS, + }, + ); + return { runs: query.data ?? [], isLoading: query.isLoading }; +} diff --git a/packages/ui/src/features/code-review/components/CloudReviewPage.tsx b/packages/ui/src/features/code-review/components/CloudReviewPage.tsx index db6e01f571..39e4a3694e 100644 --- a/packages/ui/src/features/code-review/components/CloudReviewPage.tsx +++ b/packages/ui/src/features/code-review/components/CloudReviewPage.tsx @@ -25,6 +25,9 @@ export function CloudReviewPage({ task }: CloudReviewPageProps) { const isReviewOpen = useReviewNavigationStore( (s) => (s.reviewModes[taskId] ?? "closed") !== "closed", ); + const selectedPrUrl = useReviewNavigationStore( + (s) => s.selectedPrUrls[taskId], + ); const showReviewComments = useDiffViewerStore((s) => s.showReviewComments); const { effectiveBranch, @@ -34,7 +37,7 @@ export function CloudReviewPage({ task }: CloudReviewPageProps) { reviewFiles, toolCalls, isLoading, - } = useCloudChangedFiles(taskId, task, isReviewOpen); + } = useCloudChangedFiles(taskId, task, isReviewOpen, selectedPrUrl); const { commentThreads, commentsLoading } = usePrDetails(prUrl, { includeComments: isReviewOpen && showReviewComments, }); diff --git a/packages/ui/src/features/code-review/components/ReviewPage.tsx b/packages/ui/src/features/code-review/components/ReviewPage.tsx index 4954d2ccb4..931f3d9ecb 100644 --- a/packages/ui/src/features/code-review/components/ReviewPage.tsx +++ b/packages/ui/src/features/code-review/components/ReviewPage.tsx @@ -97,6 +97,9 @@ export function ReviewPage({ task }: ReviewPageProps) { const isReviewOpen = useReviewNavigationStore( (s) => (s.reviewModes[taskId] ?? "closed") !== "closed", ); + const selectedPrUrl = useReviewNavigationStore( + (s) => s.selectedPrUrls[taskId], + ); const { effectiveSource, @@ -105,7 +108,7 @@ export function ReviewPage({ task }: ReviewPageProps) { defaultBranch, branchSourceAvailable, prSourceAvailable, - } = useEffectiveDiffSource(taskId); + } = useEffectiveDiffSource(taskId, selectedPrUrl); const showReviewComments = useDiffViewerStore((s) => s.showReviewComments); const { commentThreads, commentsLoading } = usePrDetails(prUrl, { diff --git a/packages/ui/src/features/code-review/hooks/useEffectiveDiffSource.ts b/packages/ui/src/features/code-review/hooks/useEffectiveDiffSource.ts index e9915ca5b2..1b878c6b31 100644 --- a/packages/ui/src/features/code-review/hooks/useEffectiveDiffSource.ts +++ b/packages/ui/src/features/code-review/hooks/useEffectiveDiffSource.ts @@ -23,7 +23,10 @@ export interface EffectiveDiffSource { diffStats: DiffStats; } -export function useEffectiveDiffSource(taskId: string): EffectiveDiffSource { +export function useEffectiveDiffSource( + taskId: string, + prUrlOverride?: string, +): EffectiveDiffSource { const trpc = useHostTRPC(); const repoPath = useCwd(taskId); const workspace = useWorkspace(taskId); @@ -54,7 +57,8 @@ export function useEffectiveDiffSource(taskId: string): EffectiveDiffSource { const hasLocalChanges = diffStats.filesChanged > 0; const branchSourceAvailable = !!linkedBranch && aheadOfDefault > 0; - const prUrl = useTaskPrUrl(taskId, workspace?.mode === "cloud"); + const taskPrUrl = useTaskPrUrl(taskId, workspace?.mode === "cloud"); + const prUrl = prUrlOverride ?? taskPrUrl; const prSourceAvailable = !!prUrl; const repoSlug = repoInfo @@ -62,7 +66,7 @@ export function useEffectiveDiffSource(taskId: string): EffectiveDiffSource { : null; const effectiveSource = resolveDiffSource({ - configured, + configured: prUrlOverride ? "pr" : configured, hasLocalChanges, linkedBranch, aheadOfDefault, diff --git a/packages/ui/src/features/code-review/reviewNavigationStore.test.ts b/packages/ui/src/features/code-review/reviewNavigationStore.test.ts index b6d3f413c1..f6fc0dd004 100644 --- a/packages/ui/src/features/code-review/reviewNavigationStore.test.ts +++ b/packages/ui/src/features/code-review/reviewNavigationStore.test.ts @@ -7,10 +7,27 @@ describe("reviewNavigationStore", () => { activeFilePaths: {}, scrollRequests: {}, reviewModes: {}, + selectedPrUrls: {}, commentFileFilters: {}, }); }); + it("keeps a selected PR until the review closes", () => { + const store = useReviewNavigationStore.getState(); + store.setSelectedPrUrl("task-1", "https://github.com/acme/repo/pull/2"); + store.setReviewMode("task-1", "split"); + + expect(useReviewNavigationStore.getState().selectedPrUrls["task-1"]).toBe( + "https://github.com/acme/repo/pull/2", + ); + + store.setReviewMode("task-1", "closed"); + + expect( + useReviewNavigationStore.getState().selectedPrUrls["task-1"], + ).toBeUndefined(); + }); + it("clears the comment filter when navigating to a file", () => { const store = useReviewNavigationStore.getState(); store.setCommentFileFilter("task-1", "unresolved"); diff --git a/packages/ui/src/features/code-review/reviewNavigationStore.ts b/packages/ui/src/features/code-review/reviewNavigationStore.ts index cd0bf6f739..dba01207ce 100644 --- a/packages/ui/src/features/code-review/reviewNavigationStore.ts +++ b/packages/ui/src/features/code-review/reviewNavigationStore.ts @@ -7,6 +7,7 @@ interface ReviewNavigationStoreState { activeFilePaths: Record; scrollRequests: Record; reviewModes: Record; + selectedPrUrls: Record; commentFileFilters: Record; } @@ -16,6 +17,7 @@ interface ReviewNavigationStoreActions { clearScrollRequest: (taskId: string) => void; clearTask: (taskId: string) => void; setReviewMode: (taskId: string, mode: ReviewMode) => void; + setSelectedPrUrl: (taskId: string, url: string) => void; setCommentFileFilter: (taskId: string, filter: CommentFileFilter) => void; getReviewMode: (taskId: string) => ReviewMode; } @@ -28,6 +30,7 @@ export const useReviewNavigationStore = create()( activeFilePaths: {}, scrollRequests: {}, reviewModes: {}, + selectedPrUrls: {}, commentFileFilters: {}, setActiveFilePath: (taskId, path) => @@ -57,11 +60,23 @@ export const useReviewNavigationStore = create()( ...state.commentFileFilters, [taskId]: "none", }, + selectedPrUrls: { ...state.selectedPrUrls, [taskId]: undefined }, })), setReviewMode: (taskId, mode) => set((state) => ({ reviewModes: { ...state.reviewModes, [taskId]: mode }, + // A row-selected historical PR only applies to this review visit. + // Closing restores the task's normal primary-PR resolution. + selectedPrUrls: + mode === "closed" + ? { ...state.selectedPrUrls, [taskId]: undefined } + : state.selectedPrUrls, + })), + + setSelectedPrUrl: (taskId, url) => + set((state) => ({ + selectedPrUrls: { ...state.selectedPrUrls, [taskId]: url }, })), setCommentFileFilter: (taskId, filter) => diff --git a/packages/ui/src/features/task-detail/hooks/useCloudChangedFiles.ts b/packages/ui/src/features/task-detail/hooks/useCloudChangedFiles.ts index 07e9960495..9ffd6d608d 100644 --- a/packages/ui/src/features/task-detail/hooks/useCloudChangedFiles.ts +++ b/packages/ui/src/features/task-detail/hooks/useCloudChangedFiles.ts @@ -12,9 +12,11 @@ export function useCloudChangedFiles( taskId: string, task: Task, isActive = true, + prUrlOverride?: string, ) { const cloudRunState = useCloudRunState(taskId, task); - const { prUrl, effectiveBranch, repo, isRunActive } = cloudRunState; + const prUrl = prUrlOverride ?? cloudRunState.prUrl; + const { effectiveBranch, repo, isRunActive } = cloudRunState; const { data: prFiles, @@ -45,6 +47,7 @@ export function useCloudChangedFiles( return { ...cloudRunState, + prUrl, changedFiles, remoteFiles, reviewFiles: changedFiles, From 4969440eaf4aec250d6ccecff38d60a0825f4005 Mon Sep 17 00:00:00 2001 From: Shy Alter Date: Tue, 28 Jul 2026 12:39:10 +0200 Subject: [PATCH 2/7] fix(channels): align the Activity header with the tab bar MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Activity panel's header was padding-sized (~41px) while the pane to its left — TabbedPanel's tab bar — is a fixed 32px row, so the two bottom borders never lined up. The panel also stacked a title row on top of a tabs row where the left pane has a single strip. Give ActivityPanel its own 32px header carrying the tabs directly: no separate "Activity" title row, the tab list fills the row so the line indicator lands on the bottom border, and the window controls sit right-aligned. Matches TabbedPanel and ReviewToolbar, which share the same height and border token. The shared ThreadPanelHeader is now the legacy panel's alone, so it stays exactly as it was — flag-off chrome is unchanged. Generated-By: PostHog Code Task-Id: 8ccf3555-2c68-4dbb-8b9f-1a96fd85d95a --- .../canvas/components/ActivityPanel.tsx | 91 +++++++++++++++---- .../canvas/components/ThreadPanel.tsx | 6 +- 2 files changed, 76 insertions(+), 21 deletions(-) diff --git a/packages/ui/src/features/canvas/components/ActivityPanel.tsx b/packages/ui/src/features/canvas/components/ActivityPanel.tsx index 06d075f578..67216759df 100644 --- a/packages/ui/src/features/canvas/components/ActivityPanel.tsx +++ b/packages/ui/src/features/canvas/components/ActivityPanel.tsx @@ -1,4 +1,8 @@ -import { CaretRightIcon } from "@phosphor-icons/react"; +import { + ArrowSquareOutIcon, + CaretRightIcon, + XIcon, +} from "@phosphor-icons/react"; import { Button, cn, Tabs, TabsList, TabsTrigger } from "@posthog/quill"; import { ANALYTICS_EVENTS } from "@posthog/shared/analytics-events"; import type { Task } from "@posthog/shared/domain-types"; @@ -8,7 +12,6 @@ import { TaskArtifactsList } from "@posthog/ui/features/canvas/components/TaskAr import { AgentStatusLine, ThreadLoadingState, - ThreadPanelHeader, ThreadReplyComposer, ThreadTimeline, } from "@posthog/ui/features/canvas/components/ThreadPanel"; @@ -35,27 +38,76 @@ const TABS_WITH_COMPOSER: ReadonlySet = new Set([ const TIMESTAMP_END_CLASS = "[&_[data-slot=thread-item-timestamp]]:ml-auto [&_[data-slot=thread-item-timestamp]]:shrink-0 [&_[data-slot=thread-item-timestamp]]:pl-2"; -function ActivityTabsRow({ +/** The 32px row this panel leads with: the tabs are the header, so the strip + * lines up with the tab bar of the pane on its left (TabbedPanel) and the + * review toolbar, which are the same fixed height and border. */ +function ActivityHeader({ tab, onTabChange, + onClose, + onToggleCollapsed, + onOpenFull, }: { tab: ActivityTab; onTabChange: (tab: ActivityTab) => void; + onClose?: () => void; + onToggleCollapsed?: () => void; + onOpenFull?: () => void; }) { return ( -
+
onTabChange(value as ActivityTab)} > - + {/* Nothing spells out "Activity" any more, so the strip carries the name. + The list fills the row's 32px and drops quill's 3px inset so the + line-variant indicator (bottom: 0 of the list) lands on the row's + bottom border, the way the left pane's tabs underline does. */} + {ACTIVITY_TABS.map((t) => ( - + {t.label} ))} +
+ {onOpenFull && ( + + )} + {onToggleCollapsed && ( + + )} + {onClose && ( + + )} +
); } @@ -170,13 +222,13 @@ function ActivityConversation({ TIMESTAMP_END_CLASS, )} > - - {showTaskSummary && (
@@ -235,15 +287,18 @@ export function ActivityPanel({ if (collapsed) { return ( -
- +
+ {/* Same 32px band as the expanded header, so the button doesn't jump. */} +
+ +
); } diff --git a/packages/ui/src/features/canvas/components/ThreadPanel.tsx b/packages/ui/src/features/canvas/components/ThreadPanel.tsx index 6e54d9e7f8..1070a6ae0b 100644 --- a/packages/ui/src/features/canvas/components/ThreadPanel.tsx +++ b/packages/ui/src/features/canvas/components/ThreadPanel.tsx @@ -289,9 +289,9 @@ export function ThreadLoadingState() { ); } -/** The panel's title row and window controls. Shared with ActivityPanel, which - * is the same chrome under a different title. */ -export function ThreadPanelHeader({ +/** The panel's title row and window controls. ActivityPanel has its own header + * (the tabs are its title row), so this is the legacy panel's alone. */ +function ThreadPanelHeader({ title, onClose, onToggleCollapsed, From 5dd11d87ecc6d0acbf742705fe9147d079492d59 Mon Sep 17 00:00:00 2001 From: Shy Alter Date: Tue, 28 Jul 2026 12:39:12 +0200 Subject: [PATCH 3/7] fix(channels): drop the star button from the channel header Starring a channel is already reachable from the sidebar back row's hover star and the channel list's context menu, so the header's copy was a third affordance for the same action sitting next to the channel name chip. Generated-By: PostHog Code Task-Id: 8ccf3555-2c68-4dbb-8b9f-1a96fd85d95a --- .../canvas/components/ChannelHeader.tsx | 45 ++----------------- 1 file changed, 4 insertions(+), 41 deletions(-) diff --git a/packages/ui/src/features/canvas/components/ChannelHeader.tsx b/packages/ui/src/features/canvas/components/ChannelHeader.tsx index 69dc6d35bf..8cb92f620c 100644 --- a/packages/ui/src/features/canvas/components/ChannelHeader.tsx +++ b/packages/ui/src/features/canvas/components/ChannelHeader.tsx @@ -1,54 +1,20 @@ -import { StarIcon } from "@phosphor-icons/react"; import { Button, cn } from "@posthog/quill"; -import { ANALYTICS_EVENTS } from "@posthog/shared/analytics-events"; import { ChannelTabs } from "@posthog/ui/features/canvas/components/ChannelTabs"; import { channelGlyph } from "@posthog/ui/features/canvas/components/channelGlyph"; -import { useChannelStarToggle } from "@posthog/ui/features/canvas/hooks/useChannelStars"; -import { - type Channel, - useChannels, -} from "@posthog/ui/features/canvas/hooks/useChannels"; +import { useChannels } from "@posthog/ui/features/canvas/hooks/useChannels"; import { useChannelsLayout } from "@posthog/ui/features/canvas/hooks/useChannelsLayout"; import { useMarkChannelSeen } from "@posthog/ui/features/canvas/hooks/useMarkChannelSeen"; -import { PERSONAL_CHANNEL_NAME } from "@posthog/ui/features/canvas/hooks/useTaskChannels"; -import { track } from "@posthog/ui/shell/analytics"; import { Text } from "@radix-ui/themes"; import { useNavigate, useRouterState } from "@tanstack/react-router"; -// The feed-side counterpart to the sidebar back row's hover star. -function ChannelStarButton({ channel }: { channel: Channel }) { - const { isStarred, toggleStar } = useChannelStarToggle(channel); - return ( - - ); -} - // The shared channel header. The new layout drops the section tab strip — the -// channel sidebar carries those entries — while flag off keeps it. +// channel sidebar carries those entries — while flag off keeps it. Starring +// lives on the sidebar back row and the channel list, not here. export function ChannelHeader({ channelId }: { channelId: string }) { const navigate = useNavigate(); const channelsLayout = useChannelsLayout(); const { channels } = useChannels(); - const channel = channels.find((c) => c.id === channelId); - const channelName = channel?.name; + const channelName = channels.find((c) => c.id === channelId)?.name; const pathname = useRouterState({ select: (s) => s.location.pathname }); const isHome = pathname === `/website/${channelId}`; // Every channel surface renders this header, so mark the channel read here. @@ -73,9 +39,6 @@ export function ChannelHeader({ channelId }: { channelId: string }) { {channelName ?? (channelsLayout ? "Space" : "Channel")} - {channelsLayout && channel && channel.name !== PERSONAL_CHANNEL_NAME && ( - - )} {!channelsLayout && }
); From 1b70b3a718401793f92acefee9359b4e4a4dc3f0 Mon Sep 17 00:00:00 2001 From: Shy Alter Date: Tue, 28 Jul 2026 12:39:13 +0200 Subject: [PATCH 4/7] fix(channels): show uploaded files in the task Artifacts pane MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The pane read the run artifact manifest and then dropped everything on `type === "plan"` — a type nothing in the product has ever written. The only deliverable type agents actually produce is `output`, via the `upload_artifact` tool, so the manifest branch was unreachable and files handed back by a run never appeared. Verified against a live run's manifest: 6 `output`, 2 `user_attachment`, zero `plan`. The `plan` row was speculative — down to a bare-UUID fallback for names that never arrive — so it goes rather than shipping a UI affordance for data that doesn't exist. It can come back with a producer. Same inversion is why the conversation's Files box showed these while the pane said "No artifacts yet": it filters to `output`, the exact complement. - `parseRunPlans` becomes `parseRunArtifacts(raw, types)`, and reads the `size`/`content_type`/`uploaded_at` the rows need. - Re-uploads collapse to the newest per name: agents revise a deliverable and upload it again, and keeping every copy buries the current one under its own drafts. - `formatFileSize` moves out of CloudArtifactDownloads so both surfaces format sizes the same way. Generated-By: PostHog Code Task-Id: 8d187d00-0633-4706-8443-f79130b65f9f --- .../src/canvas/runArtifactSchemas.test.ts | 64 ++++++++--- .../core/src/canvas/runArtifactSchemas.ts | 20 +++- .../components/TaskArtifactsList.test.tsx | 106 +++++++++++++++++- .../canvas/components/TaskArtifactsList.tsx | 76 +++++++------ .../components/CloudArtifactDownloads.tsx | 8 +- packages/ui/src/utils/formatFileSize.ts | 7 ++ 6 files changed, 220 insertions(+), 61 deletions(-) create mode 100644 packages/ui/src/utils/formatFileSize.ts diff --git a/packages/core/src/canvas/runArtifactSchemas.test.ts b/packages/core/src/canvas/runArtifactSchemas.test.ts index e2db2fe283..71d61af599 100644 --- a/packages/core/src/canvas/runArtifactSchemas.test.ts +++ b/packages/core/src/canvas/runArtifactSchemas.test.ts @@ -1,36 +1,68 @@ import { describe, expect, it } from "vitest"; -import { parseRunPlans } from "./runArtifactSchemas"; +import { OUTPUT_ARTIFACT_TYPES, parseRunArtifacts } from "./runArtifactSchemas"; -describe("parseRunPlans", () => { +describe("parseRunArtifacts", () => { it.each([ { name: "undefined", raw: undefined }, { name: "null", raw: null }, { name: "an object", raw: { artifacts: [] } }, - { name: "a string", raw: "plan" }, + { name: "a string", raw: "output" }, ])("reads $name as no artifacts", ({ raw }) => { - expect(parseRunPlans(raw)).toEqual([]); + expect(parseRunArtifacts(raw, OUTPUT_ARTIFACT_TYPES)).toEqual([]); }); - it("keeps only plan artifacts", () => { - const plans = parseRunPlans([ - { id: "a", type: "plan", name: "Plan A" }, - { id: "b", type: "upload", name: "internal blob" }, + // A run's manifest mixes deliverables with plumbing nobody asked to see. + it("keeps only the requested types", () => { + const outputs = parseRunArtifacts( + [ + { id: "a", type: "output", name: "report.md" }, + { id: "b", type: "user_attachment", name: "clipboard.png" }, + { id: "c", type: "skill_bundle", name: "skills.zip" }, + ], + OUTPUT_ARTIFACT_TYPES, + ); + expect(outputs).toEqual([{ id: "a", type: "output", name: "report.md" }]); + }); + + it("reads the size and upload time the row renders", () => { + const artifact = { + id: "a", + type: "output", + name: "report.md", + size: 16861, + content_type: "text/markdown", + uploaded_at: "2026-07-27T08:27:26.896719+00:00", + }; + expect(parseRunArtifacts([artifact], OUTPUT_ARTIFACT_TYPES)).toEqual([ + artifact, ]); - expect(plans).toEqual([{ id: "a", type: "plan", name: "Plan A" }]); + }); + + it("keeps an artifact that omits the optional fields", () => { + expect( + parseRunArtifacts([{ type: "output" }], OUTPUT_ARTIFACT_TYPES), + ).toEqual([{ type: "output" }]); }); // One bad entry shouldn't take the Artifacts tab down with it. it("drops entries that don't match the shape", () => { - const plans = parseRunPlans([ - { id: 42, type: "plan" }, - null, - "not an object", - { type: "plan", storage_path: "runs/1/plan.md" }, + const outputs = parseRunArtifacts( + [ + { id: 42, type: "output" }, + null, + "not an object", + { type: "output", storage_path: "runs/1/report.md" }, + ], + OUTPUT_ARTIFACT_TYPES, + ); + expect(outputs).toEqual([ + { type: "output", storage_path: "runs/1/report.md" }, ]); - expect(plans).toEqual([{ type: "plan", storage_path: "runs/1/plan.md" }]); }); it("ignores an artifact with no type", () => { - expect(parseRunPlans([{ id: "a", name: "mystery" }])).toEqual([]); + expect( + parseRunArtifacts([{ id: "a", name: "mystery" }], OUTPUT_ARTIFACT_TYPES), + ).toEqual([]); }); }); diff --git a/packages/core/src/canvas/runArtifactSchemas.ts b/packages/core/src/canvas/runArtifactSchemas.ts index 8d95e86e32..878075a68e 100644 --- a/packages/core/src/canvas/runArtifactSchemas.ts +++ b/packages/core/src/canvas/runArtifactSchemas.ts @@ -4,14 +4,30 @@ export const runArtifactSchema = z.object({ id: z.string().optional(), name: z.string().optional(), type: z.string().optional(), + size: z.number().optional(), + content_type: z.string().optional(), storage_path: z.string().optional(), + uploaded_at: z.string().optional(), }); export type RunArtifact = z.infer; -export function parseRunPlans(raw: unknown): RunArtifact[] { +/** Artifacts the agent hands back as deliverables, via the `upload_artifact` tool. */ +export const OUTPUT_ARTIFACT_TYPES = ["output"] as const; + +/** + * The run's artifacts of the given types, in manifest order. A run's manifest + * also carries plumbing the user never asked for — skill bundles, their own + * attachments — so callers name the types they want. + */ +export function parseRunArtifacts( + raw: unknown, + types: readonly string[], +): RunArtifact[] { if (!Array.isArray(raw)) return []; return raw.flatMap((entry) => { const parsed = runArtifactSchema.safeParse(entry); - return parsed.success && parsed.data.type === "plan" ? [parsed.data] : []; + if (!parsed.success) return []; + const { type } = parsed.data; + return type && types.includes(type) ? [parsed.data] : []; }); } diff --git a/packages/ui/src/features/canvas/components/TaskArtifactsList.test.tsx b/packages/ui/src/features/canvas/components/TaskArtifactsList.test.tsx index 42782087fc..88f9eb3d65 100644 --- a/packages/ui/src/features/canvas/components/TaskArtifactsList.test.tsx +++ b/packages/ui/src/features/canvas/components/TaskArtifactsList.test.tsx @@ -1,14 +1,20 @@ -import type { Task, TaskRun } from "@posthog/shared/domain-types"; +import type { Task, TaskRun, TaskRunArtifact } from "@posthog/shared"; import { fireEvent, render, screen } from "@testing-library/react"; import { beforeEach, describe, expect, it, vi } from "vitest"; const mocks = vi.hoisted(() => ({ runs: [] as TaskRun[], + presignTaskRunArtifact: vi.fn(), })); vi.mock("@posthog/ui/features/canvas/hooks/useTaskRuns", () => ({ useTaskRuns: () => ({ runs: mocks.runs, isLoading: false }), })); +vi.mock("@posthog/ui/features/auth/authClient", () => ({ + useOptionalAuthenticatedClient: () => ({ + presignTaskRunArtifact: mocks.presignTaskRunArtifact, + }), +})); vi.mock("@posthog/ui/features/git-interaction/usePrArtifact", () => ({ usePrArtifact: (url: string) => ({ safeUrl: url, @@ -33,16 +39,35 @@ const task = { latest_run: null, } as unknown as Task; -function run(id: string, prNumber: number): TaskRun { +function run( + id: string, + options: { prNumber?: number; artifacts?: Partial[] } = {}, +): TaskRun { return { id, - output: { pr_url: `https://github.com/acme/repo/pull/${prNumber}` }, + output: options.prNumber + ? { pr_url: `https://github.com/acme/repo/pull/${options.prNumber}` } + : null, + artifacts: options.artifacts, } as unknown as TaskRun; } +function outputFile( + overrides: Partial, +): Partial { + return { + type: "output", + name: "report.md", + storage_path: "runs/1/report.md", + ...overrides, + }; +} + describe("TaskArtifactsList", () => { beforeEach(() => { - mocks.runs = [run("run-1", 1), run("run-2", 2)]; + mocks.runs = [run("run-1", { prNumber: 1 }), run("run-2", { prNumber: 2 })]; + mocks.presignTaskRunArtifact.mockReset(); + mocks.presignTaskRunArtifact.mockResolvedValue("https://signed.example/x"); useReviewNavigationStore.setState({ reviewModes: {}, selectedPrUrls: {}, @@ -60,4 +85,77 @@ describe("TaskArtifactsList", () => { ); expect(state.reviewModes[task.id]).toBe("split"); }); + + it("lists the files the agent uploaded, with their size", () => { + mocks.runs = [ + run("run-1", { artifacts: [outputFile({ id: "a", size: 16861 })] }), + ]; + + render(); + + expect(screen.getByText("report.md")).toBeTruthy(); + expect(screen.getByText("File · 17 KB")).toBeTruthy(); + }); + + it("presigns the artifact of the run that produced it", () => { + mocks.runs = [ + run("run-1", { artifacts: [outputFile({ id: "a", name: "old.md" })] }), + run("run-2", { + artifacts: [ + outputFile({ + id: "b", + name: "new.md", + storage_path: "runs/2/new.md", + }), + ], + }), + ]; + + render(); + fireEvent.click(screen.getByText("new.md")); + + expect(mocks.presignTaskRunArtifact).toHaveBeenCalledWith( + "task-1", + "run-2", + "runs/2/new.md", + ); + }); + + // Agents revise a deliverable and upload it again under the same name. + it("keeps only the newest upload of a repeatedly revised file", () => { + mocks.runs = [ + run("run-1", { + artifacts: [ + outputFile({ + id: "a", + size: 1000, + uploaded_at: "2026-07-27T08:00:00+00:00", + }), + outputFile({ + id: "b", + size: 2000, + storage_path: "runs/1/report-v2.md", + uploaded_at: "2026-07-27T09:00:00+00:00", + }), + ], + }), + ]; + + render(); + + expect(screen.getAllByText("report.md")).toHaveLength(1); + expect(screen.getByText("File · 2 KB")).toBeTruthy(); + }); + + it.each([ + { name: "a plan", type: "plan" as const }, + { name: "a user attachment", type: "user_attachment" as const }, + { name: "a skill bundle", type: "skill_bundle" as const }, + ])("shows the empty state for a run with only $name", ({ type }) => { + mocks.runs = [run("run-1", { artifacts: [outputFile({ id: "a", type })] })]; + + render(); + + expect(screen.getByText("No artifacts yet")).toBeTruthy(); + }); }); diff --git a/packages/ui/src/features/canvas/components/TaskArtifactsList.tsx b/packages/ui/src/features/canvas/components/TaskArtifactsList.tsx index 0f50a76fbe..6140d42c2b 100644 --- a/packages/ui/src/features/canvas/components/TaskArtifactsList.tsx +++ b/packages/ui/src/features/canvas/components/TaskArtifactsList.tsx @@ -1,11 +1,11 @@ import { ArrowSquareOutIcon, - ClipboardTextIcon, PackageIcon, SlackLogoIcon, } from "@phosphor-icons/react"; import { - parseRunPlans, + OUTPUT_ARTIFACT_TYPES, + parseRunArtifacts, type RunArtifact, } from "@posthog/core/canvas/runArtifactSchemas"; import type { ThreadTimelineRow } from "@posthog/core/canvas/threadTimeline"; @@ -28,8 +28,10 @@ import { useReviewNavigationStore } from "@posthog/ui/features/code-review/revie import { usePrArtifact } from "@posthog/ui/features/git-interaction/usePrArtifact"; import { usePrComments } from "@posthog/ui/features/pr-review/usePrComments"; import { usePrReviewThreads } from "@posthog/ui/features/pr-review/usePrReviewThreads"; +import { FileIcon } from "@posthog/ui/primitives/FileIcon"; import { toast } from "@posthog/ui/primitives/toast"; import { openExternalUrl } from "@posthog/ui/shell/openExternal"; +import { formatFileSize } from "@posthog/ui/utils/formatFileSize"; import { parseHttpsUrl, parseShareLink } from "@posthog/ui/utils/posthogLinks"; import { navigateToShareTarget } from "@posthog/ui/utils/shareLinks"; import { getPostHogUrl } from "@posthog/ui/utils/urls"; @@ -39,23 +41,20 @@ type ArtifactRow = | { kind: "pr"; key: string; url: string } | { kind: "canvas"; key: string; name: string; url: string | null } | { - kind: "plan"; + kind: "file"; key: string; name: string; storagePath: string | null; runId: string | null; + size: number | undefined; } | { kind: "slack"; key: string; url: string }; -function readRunPlans(run: TaskRun): RunArtifact[] { - return parseRunPlans((run as { artifacts?: unknown }).artifacts); -} - -const UUID_RE = - /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}/i; -function planTitle(name: string | undefined): string { - if (!name || UUID_RE.test(name)) return "Plan"; - return name; +function readRunOutputs(run: TaskRun): RunArtifact[] { + return parseRunArtifacts( + (run as { artifacts?: unknown }).artifacts, + OUTPUT_ARTIFACT_TYPES, + ); } function buildRows( @@ -65,7 +64,6 @@ function buildRows( ): ArtifactRow[] { const rows: ArtifactRow[] = []; const seenPrUrls = new Set(); - const seenPlanKeys = new Set(); const addPr = (url: string, key: string) => { if (seenPrUrls.has(url)) return; @@ -89,24 +87,35 @@ function buildRows( const allRuns = runs.length > 0 ? runs : task.latest_run ? [task.latest_run] : []; + + // Re-uploading a file replaces it rather than adding a second one: agents + // revise a deliverable and upload it again under the same name, so keeping + // every copy would bury the current one under its own drafts. + const newestByName = new Map(); for (const run of allRuns) { const outputPr = run.output?.pr_url; if (typeof outputPr === "string" && outputPr) { addPr(outputPr, `output-pr:${outputPr}`); } - for (const plan of readRunPlans(run)) { - const key = `plan:${plan.id ?? plan.storage_path ?? plan.name}`; - if (seenPlanKeys.has(key)) continue; - seenPlanKeys.add(key); - rows.push({ - kind: "plan", - key, - name: planTitle(plan.name), - storagePath: plan.storage_path ?? null, - runId: run.id, - }); + for (const file of readRunOutputs(run)) { + if (!file.name) continue; + const previous = newestByName.get(file.name); + const isNewer = + !previous || + (file.uploaded_at ?? "") >= (previous.file.uploaded_at ?? ""); + if (isNewer) newestByName.set(file.name, { file, runId: run.id }); } } + for (const [name, { file, runId }] of newestByName) { + rows.push({ + kind: "file", + key: `file:${file.id ?? file.storage_path ?? name}`, + name, + storagePath: file.storage_path ?? null, + runId, + size: file.size, + }); + } const slackUrl = task.latest_run?.state?.slack_thread_url; if (typeof slackUrl === "string" && slackUrl) { @@ -226,16 +235,18 @@ function CanvasRow({ name, url }: { name: string; url: string | null }) { ); } -function PlanRow({ +function FileRow({ taskId, runId, name, storagePath, + size, }: { taskId: string; runId: string | null; name: string; storagePath: string | null; + size: number | undefined; }) { const client = useOptionalAuthenticatedClient(); const canOpen = !!client && !!runId && !!storagePath; @@ -249,7 +260,7 @@ function PlanRow({ ) .then((url) => openExternalUrl(url)) .catch((error: unknown) => { - toast.error("Couldn't open plan", { + toast.error("Couldn't open file", { description: error instanceof Error ? error.message : String(error), }); @@ -258,9 +269,9 @@ function PlanRow({ : undefined; return ( } + icon={} title={name} - detail="Plan" + detail={["File", formatFileSize(size)].filter(Boolean).join(" · ")} external={canOpen} onOpen={onOpen} /> @@ -289,8 +300,8 @@ export function TaskArtifactsList({ No artifacts yet - Pull requests and canvases produced while working on this task show - up here. + Pull requests, canvases, and files produced while working on this + task show up here. @@ -304,13 +315,14 @@ export function TaskArtifactsList({ ) : row.kind === "canvas" ? ( - ) : row.kind === "plan" ? ( - ) : ( Date: Tue, 28 Jul 2026 12:39:15 +0200 Subject: [PATCH 5/7] test(channels): pin artifact rows to per-file-type icons MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The rows already use the same FileIcon the chat's file list does, so markdown resolves to the markdown icon and HTML to the HTML one. Lock that in — the row it replaced hardcoded a single clipboard glyph, and nothing else would catch a regression back to a generic icon. Generated-By: PostHog Code Task-Id: 8d187d00-0633-4706-8443-f79130b65f9f --- .../components/TaskArtifactsList.test.tsx | 38 ++++++++++++------- .../canvas/components/TaskArtifactsList.tsx | 36 +++++++----------- 2 files changed, 38 insertions(+), 36 deletions(-) diff --git a/packages/ui/src/features/canvas/components/TaskArtifactsList.test.tsx b/packages/ui/src/features/canvas/components/TaskArtifactsList.test.tsx index 88f9eb3d65..878222dd48 100644 --- a/packages/ui/src/features/canvas/components/TaskArtifactsList.test.tsx +++ b/packages/ui/src/features/canvas/components/TaskArtifactsList.test.tsx @@ -4,16 +4,14 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; const mocks = vi.hoisted(() => ({ runs: [] as TaskRun[], - presignTaskRunArtifact: vi.fn(), + openArtifactTab: vi.fn(), })); vi.mock("@posthog/ui/features/canvas/hooks/useTaskRuns", () => ({ useTaskRuns: () => ({ runs: mocks.runs, isLoading: false }), })); -vi.mock("@posthog/ui/features/auth/authClient", () => ({ - useOptionalAuthenticatedClient: () => ({ - presignTaskRunArtifact: mocks.presignTaskRunArtifact, - }), +vi.mock("@posthog/ui/features/panels/panelLayoutStore", () => ({ + usePanelLayoutStore: () => mocks.openArtifactTab, })); vi.mock("@posthog/ui/features/git-interaction/usePrArtifact", () => ({ usePrArtifact: (url: string) => ({ @@ -66,8 +64,7 @@ function outputFile( describe("TaskArtifactsList", () => { beforeEach(() => { mocks.runs = [run("run-1", { prNumber: 1 }), run("run-2", { prNumber: 2 })]; - mocks.presignTaskRunArtifact.mockReset(); - mocks.presignTaskRunArtifact.mockResolvedValue("https://signed.example/x"); + mocks.openArtifactTab.mockReset(); useReviewNavigationStore.setState({ reviewModes: {}, selectedPrUrls: {}, @@ -97,7 +94,22 @@ describe("TaskArtifactsList", () => { expect(screen.getByText("File · 17 KB")).toBeTruthy(); }); - it("presigns the artifact of the run that produced it", () => { + // The row should read like the chat's file list: markdown looks like + // markdown, HTML looks like HTML. + it.each([ + { name: "notes.md", icon: "markdown" }, + { name: "demo.html", icon: "html" }, + ])("gives $name an icon for its file type", ({ name, icon }) => { + mocks.runs = [run("run-1", { artifacts: [outputFile({ id: "a", name })] })]; + + const { container } = render( + , + ); + + expect(container.querySelector("img")?.getAttribute("src")).toContain(icon); + }); + + it("opens the artifact of the run that produced it beside the chat", () => { mocks.runs = [ run("run-1", { artifacts: [outputFile({ id: "a", name: "old.md" })] }), run("run-2", { @@ -114,11 +126,11 @@ describe("TaskArtifactsList", () => { render(); fireEvent.click(screen.getByText("new.md")); - expect(mocks.presignTaskRunArtifact).toHaveBeenCalledWith( - "task-1", - "run-2", - "runs/2/new.md", - ); + expect(mocks.openArtifactTab).toHaveBeenCalledWith("task-1", { + runId: "run-2", + artifactId: "b", + name: "new.md", + }); }); // Agents revise a deliverable and upload it again under the same name. diff --git a/packages/ui/src/features/canvas/components/TaskArtifactsList.tsx b/packages/ui/src/features/canvas/components/TaskArtifactsList.tsx index 6140d42c2b..4ac3e526f4 100644 --- a/packages/ui/src/features/canvas/components/TaskArtifactsList.tsx +++ b/packages/ui/src/features/canvas/components/TaskArtifactsList.tsx @@ -21,15 +21,14 @@ import type { TaskRun, TaskThreadMessage, } from "@posthog/shared/domain-types"; -import { useOptionalAuthenticatedClient } from "@posthog/ui/features/auth/authClient"; import { iconForTemplate } from "@posthog/ui/features/canvas/components/canvasTemplateIcon"; import { useTaskRuns } from "@posthog/ui/features/canvas/hooks/useTaskRuns"; import { useReviewNavigationStore } from "@posthog/ui/features/code-review/reviewNavigationStore"; import { usePrArtifact } from "@posthog/ui/features/git-interaction/usePrArtifact"; +import { usePanelLayoutStore } from "@posthog/ui/features/panels/panelLayoutStore"; import { usePrComments } from "@posthog/ui/features/pr-review/usePrComments"; import { usePrReviewThreads } from "@posthog/ui/features/pr-review/usePrReviewThreads"; import { FileIcon } from "@posthog/ui/primitives/FileIcon"; -import { toast } from "@posthog/ui/primitives/toast"; import { openExternalUrl } from "@posthog/ui/shell/openExternal"; import { formatFileSize } from "@posthog/ui/utils/formatFileSize"; import { parseHttpsUrl, parseShareLink } from "@posthog/ui/utils/posthogLinks"; @@ -43,8 +42,8 @@ type ArtifactRow = | { kind: "file"; key: string; + artifactId: string | null; name: string; - storagePath: string | null; runId: string | null; size: number | undefined; } @@ -110,8 +109,8 @@ function buildRows( rows.push({ kind: "file", key: `file:${file.id ?? file.storage_path ?? name}`, + artifactId: file.id ?? null, name, - storagePath: file.storage_path ?? null, runId, size: file.size, }); @@ -238,33 +237,25 @@ function CanvasRow({ name, url }: { name: string; url: string | null }) { function FileRow({ taskId, runId, + artifactId, name, - storagePath, size, }: { taskId: string; runId: string | null; + artifactId: string | null; name: string; - storagePath: string | null; size: number | undefined; }) { - const client = useOptionalAuthenticatedClient(); - const canOpen = !!client && !!runId && !!storagePath; + const openArtifactTab = usePanelLayoutStore((state) => state.openArtifactTab); + const canOpen = !!runId && !!artifactId; const onOpen = canOpen ? () => { - client - .presignTaskRunArtifact( - taskId, - runId as string, - storagePath as string, - ) - .then((url) => openExternalUrl(url)) - .catch((error: unknown) => { - toast.error("Couldn't open file", { - description: - error instanceof Error ? error.message : String(error), - }); - }); + openArtifactTab(taskId, { + runId: runId as string, + artifactId: artifactId as string, + name, + }); } : undefined; return ( @@ -272,7 +263,6 @@ function FileRow({ icon={} title={name} detail={["File", formatFileSize(size)].filter(Boolean).join(" · ")} - external={canOpen} onOpen={onOpen} /> ); @@ -320,8 +310,8 @@ export function TaskArtifactsList({ key={row.key} taskId={task.id} runId={row.runId} + artifactId={row.artifactId} name={row.name} - storagePath={row.storagePath} size={row.size} /> ) : ( From 2472d828456f47a35cb934eafd6c34879a7b1e5d Mon Sep 17 00:00:00 2001 From: Adam Leith Date: Tue, 28 Jul 2026 11:36:52 +0100 Subject: [PATCH 6/7] feat(spaces): drive the sidebar list from the search box MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The spaces list was a plain tree with a filter box bolted on: the only way to reach a row was the mouse, and the box could not be cleared without one either. Every row is now an Autocomplete option, filtered or not, so the arrows and Enter work the moment the pane opens rather than only once you have typed something. Rows keep their hover menus, context menu and dialogs — SpaceRowSurface renders quill's Button off the layout and an AutocompleteItem on it, so only the click and highlight routing changes. Notes on the pieces that are not obvious: - `items` carries the row values in DOM order even though the rows render as elements. Without it the highlight has no list to index into, and the first ArrowDown after a keystroke is swallowed re-establishing the highlight it is already showing. - `open` rather than `defaultOpen`: picking a row closes an ordinary combobox, and a closed one stops answering the arrow keys, so coming back from a space found the list dead. - `keepHighlight`: Base UI resets the highlight on pointer-leave and `autoHighlight="always"` re-seeds it at the top, so drifting the mouse across the gap between two rows threw the keyboard back to #me. - Collapsed groups unmount under the layout. A kept-mounted collapsed row is still an option, and the arrows would walk onto spaces that were folded away. - quill sizes its list as a popup (a ~250px cap, its own padding) and ships it unlayered, so the overrides that make the list fill the pane have to be important to outrank it. Returning from a space focuses the box, selects any stale query, and sends Home so the list reopens at the top — Autocomplete otherwise leaves the highlight on the row you opened and exposes no way to move it. Escape clears the query and the clear button gets a real tab stop; Base UI's is a tabIndex=-1 decoration. Co-Authored-By: Claude Opus 5 (1M context) --- .../canvas/components/ChannelsList.test.tsx | 195 ++++++- .../canvas/components/ChannelsList.tsx | 498 +++++++++++++----- 2 files changed, 556 insertions(+), 137 deletions(-) diff --git a/packages/ui/src/features/canvas/components/ChannelsList.test.tsx b/packages/ui/src/features/canvas/components/ChannelsList.test.tsx index 0109a40b25..56e00378ee 100644 --- a/packages/ui/src/features/canvas/components/ChannelsList.test.tsx +++ b/packages/ui/src/features/canvas/components/ChannelsList.test.tsx @@ -1,5 +1,5 @@ import { Theme } from "@radix-ui/themes"; -import { render, screen } from "@testing-library/react"; +import { act, render, screen, waitFor } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { beforeEach, describe, expect, it, vi } from "vitest"; @@ -48,6 +48,11 @@ vi.mock("@tanstack/react-router", () => ({ useRouterState: () => "/website", })); +import { + showChannelList, + showChannelPane, +} from "@posthog/ui/features/canvas/stores/channelPaneStore"; +import { useSidebarStore } from "@posthog/ui/features/sidebar/sidebarStore"; import { ChannelsList } from "./ChannelsList"; const ME = { id: "me-id", name: "me", path: "/me" }; @@ -68,6 +73,12 @@ describe("ChannelsList", () => { mocks.channels = [ME, ENG, DESIGN]; mocks.starredPaths = []; mocks.channelsLayout = true; + // The pane store is module state: reset to its resting value so a test that + // slides the slider can't hand the next one a pre-focused search box. + showChannelPane(); + // Same for the collapse state — a test that folds a group away would + // otherwise hide its rows from every test that runs after it. + useSidebarStore.setState({ collapsedSections: new Set() }); }); it("pins #me above the channels, with its ⌘1 shortcut", () => { @@ -156,5 +167,187 @@ describe("ChannelsList", () => { expect(screen.getByText(/No spaces match/)).toBeTruthy(); }); + + // Searching is a keyboard flow start to finish: you never leave the input + // to reach the row you just named. + it("opens the highlighted result on Enter", async () => { + const user = userEvent.setup(); + renderList(); + + await user.type(screen.getByLabelText("Search spaces"), "eng"); + await user.keyboard("{Enter}"); + + expect(mocks.navigate).toHaveBeenCalledWith({ + to: "/website/$channelId", + params: { channelId: ENG.id }, + }); + }); + + it("moves the highlight with the arrow keys", async () => { + const user = userEvent.setup(); + renderList(); + + // "me", "design" and "engineering" all contain an "e"; the first is + // highlighted to begin with, so one press down lands on the second. + await user.type(screen.getByLabelText("Search spaces"), "e"); + await user.keyboard("{ArrowDown}{Enter}"); + + expect(mocks.navigate).toHaveBeenCalledWith({ + to: "/website/$channelId", + params: { channelId: ENG.id }, + }); + }); + + // Base UI's clear button is a tabIndex=-1 decoration by default, which left + // the keyboard with no way out of a query. + it("clears on Escape", async () => { + const user = userEvent.setup(); + renderList(); + const input = screen.getByLabelText("Search spaces"); + + await user.type(input, "eng"); + expect(screen.queryByText("design")).toBeNull(); + + await user.keyboard("{Escape}"); + + expect((input as HTMLInputElement).value).toBe(""); + expect(screen.getByText("design")).toBeTruthy(); + }); + + it("gives the clear button a tab stop", async () => { + const user = userEvent.setup(); + renderList(); + + await user.type(screen.getByLabelText("Search spaces"), "eng"); + + const clear = screen.getByLabelText("Clear search"); + expect((clear as HTMLElement).tabIndex).toBe(0); + + await user.tab(); + expect(document.activeElement).toBe(clear); + await user.keyboard("{Enter}"); + expect( + (screen.getByLabelText("Search spaces") as HTMLInputElement).value, + ).toBe(""); + }); + }); + + // The list is a switcher first: arrowing to a space has to work the moment + // the pane opens, not only once there's something to filter by. + describe("keyboard traversal with no query", () => { + it("walks the rows and opens the highlighted one", async () => { + const user = userEvent.setup(); + renderList(); + + await user.click(screen.getByLabelText("Search spaces")); + await user.keyboard("{ArrowDown}{Enter}"); + + expect(mocks.navigate).toHaveBeenCalledWith({ + to: "/website/$channelId", + params: { channelId: ENG.id }, + }); + }); + + // Base UI resets the highlight when the pointer leaves a row, and + // `autoHighlight="always"` then snaps it back to the top — so drifting the + // mouse across the gap between two rows threw the keyboard back to #me. + it("keeps the highlight when the pointer leaves a row", async () => { + const user = userEvent.setup(); + renderList(); + + await user.click(screen.getByLabelText("Search spaces")); + const row = screen.getByText("engineering"); + await user.hover(row); + await user.unhover(row); + await user.keyboard("{Enter}"); + + expect(mocks.navigate).toHaveBeenCalledWith({ + to: "/website/$channelId", + params: { channelId: ENG.id }, + }); + }); + + // A kept-mounted collapsed row would still be an option, so ↓ would walk + // onto spaces that were folded away. + it("drops a collapsed group's rows from the list", async () => { + const user = userEvent.setup(); + renderList(); + expect(screen.getByText("engineering")).toBeTruthy(); + + await user.click(screen.getByText("Spaces")); + + expect(screen.queryByText("engineering")).toBeNull(); + }); + }); + + // Sliding back from a space, the list is what you came here for — so it hands + // the search box the caret rather than making you click it. + describe("focus on returning to the list", () => { + it("focuses the search box when the pane slides back", async () => { + renderList(); + expect(document.activeElement).not.toBe( + screen.getByLabelText("Search spaces"), + ); + + act(() => showChannelList()); + + await waitFor(() => + expect(document.activeElement).toBe( + screen.getByLabelText("Search spaces"), + ), + ); + }); + + // Opening a row would close an ordinary combobox, and a closed one stops + // answering the arrow keys — the pane came back with its keyboard dead and + // its highlight parked on the space you had just left. + it("returns the highlight to the top with the arrows live", async () => { + const user = userEvent.setup(); + renderList(); + + await user.click(screen.getByText("engineering")); + act(() => showChannelList()); + await waitFor(() => + expect(document.activeElement).toBe( + screen.getByLabelText("Search spaces"), + ), + ); + mocks.navigate.mockClear(); + + // From the top, one press down is "engineering" again. Left where it was, + // it would have been the row after it. + await user.keyboard("{ArrowDown}{Enter}"); + + expect(mocks.navigate).toHaveBeenCalledWith({ + to: "/website/$channelId", + params: { channelId: ENG.id }, + }); + }); + + it("selects a stale query so the next keystroke replaces it", async () => { + const user = userEvent.setup(); + renderList(); + const input = screen.getByLabelText("Search spaces") as HTMLInputElement; + + await user.type(input, "eng"); + await user.click(screen.getByText("engineering")); + act(() => showChannelList()); + + await waitFor(() => expect(document.activeElement).toBe(input)); + expect(input.selectionStart).toBe(0); + expect(input.selectionEnd).toBe("eng".length); + }); + + it("leaves focus alone off the channels layout", async () => { + mocks.channelsLayout = false; + renderList(); + + act(() => showChannelList()); + + await waitFor(() => + expect(screen.queryByLabelText("Search spaces")).toBeNull(), + ); + expect(document.activeElement).toBe(document.body); + }); }); }); diff --git a/packages/ui/src/features/canvas/components/ChannelsList.tsx b/packages/ui/src/features/canvas/components/ChannelsList.tsx index 5b7eac01e1..abd850d870 100644 --- a/packages/ui/src/features/canvas/components/ChannelsList.tsx +++ b/packages/ui/src/features/canvas/components/ChannelsList.tsx @@ -19,6 +19,11 @@ import { AlertDialogFooter, AlertDialogHeader, AlertDialogTitle, + Autocomplete, + AutocompleteClear, + AutocompleteInput, + AutocompleteItem, + AutocompleteList, Button, ButtonGroup, AlertDialog as ConfirmDialog, @@ -35,7 +40,6 @@ import { DropdownMenuTrigger, Empty, EmptyHeader, - Input, Kbd, MenuLabel, Tooltip, @@ -65,7 +69,10 @@ import { useTaskChannels, } from "@posthog/ui/features/canvas/hooks/useTaskChannels"; import { useIsChannelUnread } from "@posthog/ui/features/canvas/hooks/useUnreadChannels"; -import { showChannelPane } from "@posthog/ui/features/canvas/stores/channelPaneStore"; +import { + showChannelPane, + useChannelPaneStore, +} from "@posthog/ui/features/canvas/stores/channelPaneStore"; import { resetCurrentChannel, useCurrentChannelStore, @@ -82,9 +89,76 @@ import { openTaskInput } from "@posthog/ui/router/useOpenTask"; import { track } from "@posthog/ui/shell/analytics"; import { Box, Flex } from "@radix-ui/themes"; import { useNavigate, useRouterState } from "@tanstack/react-router"; -import { Fragment, type ReactNode, useState } from "react"; +import { + type ComponentProps, + Fragment, + type ReactNode, + useEffect, + useRef, + useState, +} from "react"; import { hostClient } from "../hostClient"; +/** + * A row's clickable surface. + * + * Under the layout every row is an Autocomplete option, so ↑/↓/⏎ walk the list + * whether or not there's a query — the search box is the only thing that ever + * holds focus, and the list is what it drives. Off the layout there is no + * search box to drive anything, so the rows stay plain buttons. + * + * Both render the same quill Button underneath; the option just routes its + * clicks and highlight through Autocomplete. Rest props are forwarded because + * this is handed to `ContextMenuTrigger` as its rendered element. + */ +function SpaceRowSurface({ + asOption, + optionValue, + className, + children, + ...rest +}: ComponentProps & { + asOption: boolean; + /** Identifies the row to Autocomplete; unused off the layout. */ + optionValue: string; +}) { + if (!asOption) { + return ( + + ); + } + return ( + span]:w-full [&>span]:gap-2", + className, + )} + // The two branches take the same handlers typed against different + // elements — quill's option renders a button of its own, so what the + // callers pass is a button's props either way. + {...(rest as ComponentProps)} + > + {children} + + ); +} + // One actionable entry in a channel's menu, rendered the same whether it // surfaces in the hover "..." dropdown or the right-click context menu. type ChannelActionItem = { @@ -328,9 +402,8 @@ function ChannelSection({ }) { const spacesLayout = useChannelsLayout(); const noun = spacesLayout ? "space" : "channel"; - const navigate = useNavigate(); const pathname = useRouterState({ select: (s) => s.location.pathname }); - const setCurrentChannel = useCurrentChannelStore((s) => s.setCurrentChannel); + const openChannel = useOpenChannel(); const base = `/website/${channel.id}`; // Highlight the row whenever any of the channel's routes is open. const isActive = pathname === base || pathname.startsWith(`${base}/`); @@ -360,28 +433,12 @@ function ChannelSection({ { - track(ANALYTICS_EVENTS.CHANNEL_ACTION, { - action_type: "nav_click", - surface: "sidebar", - channel_id: channel.id, - }); - // Slide before navigating: the route effect would get there - // too, but not until the navigation resolves. - showChannelPane(); - setCurrentChannel(channel.id); - void navigate({ - to: "/website/$channelId", - params: { channelId: channel.id }, - }); - }} + onClick={() => openChannel(channel)} {...focusProps} - className="w-full min-w-0 justify-start gap-2 data-selected:bg-fill-selected data-selected:text-foreground" > {channelGlyph(channel.name, { size: 14, @@ -415,7 +472,7 @@ function ChannelSection({ {formatHotkey(`mod+${hotkeySlot}`)} )} - + } /> @@ -552,29 +609,24 @@ function ChannelSection({ // The feed and task ownership live on the per-user backend personal channel; // the "me" folder is the bridge that keeps the folder-keyed surfaces // (CONTEXT.md, artifacts) routable, created lazily on first open. -function PersonalChannelRow({ hotkeySlot }: { hotkeySlot?: number }) { +/** + * Opening the "me" row, shared by the row itself and the search results. + * + * The folder is created on first use, so every action resolves the id rather + * than closing over it — "me" is actionable before it exists. The create is + * shared (ensurePersonalChannel) so a row click racing its "+" menu can't + * provision two. + */ +function useOpenPersonalChannel(): { + ensureFolderId: () => Promise; + openPersonalChannel: () => Promise; + isCreating: boolean; +} { const navigate = useNavigate(); - const pathname = useRouterState({ select: (s) => s.location.pathname }); const setCurrentChannel = useCurrentChannelStore((s) => s.setCurrentChannel); const { channels } = useChannels(); const { createChannel, isCreating } = useChannelMutations(); - // Listing backend channels lazily provisions the personal channel server-side. - useTaskChannels(); - // The "+" dropdown (New task / New canvas), mirroring a shared channel row. - const [newMenuOpen, setNewMenuOpen] = useState(false); - const isUnread = useIsChannelUnread()(PERSONAL_CHANNEL_NAME); - const meFolder = channels.find((c) => c.name === PERSONAL_CHANNEL_NAME); - const createAndOpenCanvas = useCreateAndOpenDashboard(meFolder?.id); - const isActive = - !!meFolder && - (pathname === `/website/${meFolder.id}` || - pathname.startsWith(`/website/${meFolder.id}/`)); - - // The "me" folder is created on first use, so every action resolves the id - // rather than closing over it — the row is actionable before it exists. The - // create is shared (ensurePersonalChannel) so a row click racing its "+" menu - // can't provision two. const ensureFolderId = async (): Promise => { try { return (await ensurePersonalChannel(channels, createChannel)).id; @@ -586,7 +638,7 @@ function PersonalChannelRow({ hotkeySlot }: { hotkeySlot?: number }) { } }; - const open = async () => { + const openPersonalChannel = async () => { const channelId = await ensureFolderId(); if (!channelId) return; showChannelPane(); @@ -594,6 +646,54 @@ function PersonalChannelRow({ hotkeySlot }: { hotkeySlot?: number }) { void navigate({ to: "/website/$channelId", params: { channelId } }); }; + return { ensureFolderId, openPersonalChannel, isCreating }; +} + +/** + * Navigating into a channel, shared by the tree rows and the search results. + * Slides before navigating: the route effect would get there too, but not until + * the navigation resolves. + */ +function useOpenChannel(): (channel: Channel) => void { + const navigate = useNavigate(); + const setCurrentChannel = useCurrentChannelStore((s) => s.setCurrentChannel); + + return (channel: Channel) => { + track(ANALYTICS_EVENTS.CHANNEL_ACTION, { + action_type: "nav_click", + surface: "sidebar", + channel_id: channel.id, + }); + showChannelPane(); + setCurrentChannel(channel.id); + void navigate({ + to: "/website/$channelId", + params: { channelId: channel.id }, + }); + }; +} + +function PersonalChannelRow({ hotkeySlot }: { hotkeySlot?: number }) { + const spacesLayout = useChannelsLayout(); + const pathname = useRouterState({ select: (s) => s.location.pathname }); + const { channels } = useChannels(); + const { ensureFolderId, openPersonalChannel, isCreating } = + useOpenPersonalChannel(); + // Listing backend channels lazily provisions the personal channel server-side. + useTaskChannels(); + // The "+" dropdown (New task / New canvas), mirroring a shared channel row. + const [newMenuOpen, setNewMenuOpen] = useState(false); + const isUnread = useIsChannelUnread()(PERSONAL_CHANNEL_NAME); + + const meFolder = channels.find((c) => c.name === PERSONAL_CHANNEL_NAME); + const createAndOpenCanvas = useCreateAndOpenDashboard(meFolder?.id); + const isActive = + !!meFolder && + (pathname === `/website/${meFolder.id}` || + pathname.startsWith(`/website/${meFolder.id}/`)); + + const open = openPersonalChannel; + const newTask = async () => { const channelId = await ensureFolderId(); if (!channelId) return; @@ -618,14 +718,14 @@ function PersonalChannelRow({ hotkeySlot }: { hotkeySlot?: number }) { return ( - +
@@ -722,6 +822,7 @@ function ChannelGroup({ label, className, flat, + keepMounted = true, children, }: { sectionId: string; @@ -729,6 +830,12 @@ function ChannelGroup({ className?: string; /** Layout-only: rows sit at the label's level instead of indented under it. */ flat?: boolean; + /** + * Off under the layout: a kept-mounted collapsed row is still an Autocomplete + * option, so ↓ would walk onto spaces the user has folded away. Paying the + * rebuild on expand is better than highlighting a row nobody can see. + */ + keepMounted?: boolean; children: ReactNode; }) { const collapsedSections = useSidebarStore((s) => s.collapsedSections); @@ -777,7 +884,7 @@ function ChannelGroup({ dropdown, a tooltip and two dialogs up front, so unmounting on close makes each expand rebuild the lot (~940ms for 46 channels, vs ~80ms to collapse). */} - +
{children}
 @@ -822,95 +929,214 @@ export function ChannelsList() { const noMatches = normalizedQuery !== "" && !meMatches && !searchResults.length; - return ( - // One shared provider groups every row tooltip so that once one shows, - // moving to the next row reveals its tooltip instantly (no re-delay). - - - {channelsLayout && ( - - setQuery(event.target.value)} - placeholder="Search spaces…" - aria-label="Search spaces" - className="h-7 text-[13px]" + // The option values, in the order the rows below render them. The rows are + // elements rather than a rendered collection, but Autocomplete still needs the + // list to map a highlight index onto — without it the first ArrowDown after a + // keystroke is swallowed re-establishing the highlight it already shows. + // A collapsed group renders no rows, so it contributes none. + const collapsedSections = useSidebarStore((s) => s.collapsedSections); + // "me" is provisioned on first use; before it exists it has no id to go by. + const meValue = me?.id ?? PERSONAL_CHANNEL_NAME; + const optionValues = normalizedQuery + ? [ + ...(meMatches ? [meValue] : []), + ...searchResults.map((channel) => channel.id), + ] + : [ + meValue, + ...(collapsedSections.has(STARRED_SECTION_ID) + ? [] + : starred.map((channel) => channel.id)), + ...(collapsedSections.has(CHANNELS_SECTION_ID) + ? [] + : others.map((channel) => channel.id)), + ]; + + // Coming back from a space, the list is what you came here to browse — so the + // search box takes focus and any previous query is selected, ready to be typed + // over. Only on the transition: a cold start rests on the channel pane, and + // re-focusing on every render would steal focus from the rows themselves. + const pane = useChannelPaneStore((s) => s.pane); + const previousPane = useRef(pane); + const searchRef = useRef(null); + useEffect(() => { + const cameFromChannel = previousPane.current === "channel"; + previousPane.current = pane; + if (!channelsLayout || pane !== "list" || !cameFromChannel) return; + const input = searchRef.current; + if (!input) return; + input.focus(); + // Autocomplete leaves its highlight on the row you opened and exposes no + // way to move it, so the pane would come back mid-list. Home is the key it + // listens for; sending it is how the list reopens at the top. + input.dispatchEvent( + new KeyboardEvent("keydown", { key: "Home", bubbles: true }), + ); + // After Home, so the caret it parks at the start doesn't undo the selection. + input.select(); + }, [pane, channelsLayout]); + + const rows = normalizedQuery ? ( + <> + {meMatches && } + {searchResults.map((channel) => ( + + ))} + {noMatches && ( + + + No {channelsLayout ? "spaces" : "channels"} match “{query.trim()}”. + + + )} + + ) : ( + <> + + + {starred.length > 0 && ( + + {starred.map((channel) => ( + - + ))} + + )} + + + {!isLoading && channels.length === 0 && ( + + + No {channelsLayout ? "spaces" : "channels"} yet. + + )} - {/* Bottom padding clears the floating create button (ChannelsFab), so - the last channel stays reachable at full scroll. */} - - {normalizedQuery ? ( - <> - {meMatches && } - {searchResults.map((channel) => ( - - ))} - {noMatches && ( - - - No {channelsLayout ? "spaces" : "channels"} match “ - {query.trim()}”. - - - )} - - ) : ( - <> - + {others.map((channel) => ( + + ))} + + + ); - {starred.length > 0 && ( - - {starred.map((channel) => ( - - ))} - - )} + // Bottom padding clears the floating create button (ChannelsFab), so the last + // channel stays reachable at full scroll. + const scrollClass = + "scroll-mask-4 min-h-0 flex-1 overflow-y-auto px-2 pt-2 pb-16"; + // quill sizes its list as a popup — a ~250px cap and its own 4px padding — + // and ships it unlayered, so plain utilities lose to it however they're + // ordered. Here the list *is* the pane, so the cap has to go and the pane's + // own padding has to win: `!` is what outranks an unlayered rule. + const listClass = cn( + "flex flex-col gap-px", + "!max-h-none !px-2 !pt-2 !pb-16", + scrollClass, + ); - - {!isLoading && channels.length === 0 && ( - - - No {channelsLayout ? "spaces" : "channels"} yet. - - - )} - {others.map((channel) => ( - - ))} - - - )} + const body = ( + + {channelsLayout && ( + + { + // Base UI's clear is a tabIndex=-1 decoration, so Escape is the + // keyboard way out of a query. With the box already empty there's + // nothing to clear, and Escape belongs to whoever is listening + // above (closing the sidebar, dismissing a dialog). + if (event.key !== "Escape" || query === "") return; + event.preventDefault(); + event.stopPropagation(); + setQuery(""); + }} + > + {/* Rendered here rather than via `showClear` so it can be given a + tab stop: quill passes no props to the one it renders itself. */} + setQuery("")} + /> + + + )} + {channelsLayout ? ( + // Every row is an option, filtered or not, so ↑/↓/⏎ work the moment the + // pane opens rather than only once you've typed something. + {rows} + ) : ( + + {rows} - + )} + + ); + + return ( + // One shared provider groups every row tooltip so that once one shows, + // moving to the next row reveals its tooltip instantly (no re-delay). + + {channelsLayout ? ( + // The rows render as elements — they're a tree of collapsible groups, + // not a flat collection — so `items` carries their values alone, in the + // same order. Filtering is ours (hence `filter={null}`; Base UI's matcher + // would run over an already-narrowed set). `inline` renders the list in + // the pane instead of a popup, and `defaultOpen` keeps it rendered + // without a trigger to open it. + + inline + // Pinned open, not `defaultOpen`: picking a row closes an ordinary + // combobox, and a closed one stops answering the arrow keys. This list + // is the pane itself — there is nothing to close, and coming back from + // a space has to find it live. + open + items={optionValues} + filter={null} + value={query} + autoHighlight="always" + // Without this the highlight resets on pointer-leave, and "always" + // then snaps it back to the first row — so drifting the mouse across + // the gap between two rows threw the keyboard back to the top. + keepHighlight + onValueChange={(value, eventDetails) => { + // Selecting a row would otherwise write the row's value back into + // the input; only what the user types moves the query. + if (eventDetails.reason !== "input-change") return; + if (typeof value === "string") setQuery(value); + }} + > + {body} + + ) : ( + body + )} ); } From c7ca9568d0fe2111469783c0f790889a0b8fef74 Mon Sep 17 00:00:00 2001 From: Adam Leith Date: Tue, 28 Jul 2026 11:48:54 +0100 Subject: [PATCH 7/7] refactor(spaces): draw the sidebar's loading rows with SkeletonText MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The channel pane's placeholder rows were fixed-height pills that sat at their own scale rather than the rows' — a stack of near-white bars that didn't read as the list they become. SkeletonText sizes its bar off the line it stands in, so giving each row the type scale of the title it replaces (and the label bar the scale of the MenuLabel above it) makes the placeholder the shape of the content. Row widths move from Tailwind fractions to the percentages the primitive takes. The item glyph stays a plain Skeleton: it is a square, not text. Co-Authored-By: Claude Opus 5 (1M context) --- .../canvas/components/ChannelSidebar.tsx | 29 ++++++++++++------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/packages/ui/src/features/canvas/components/ChannelSidebar.tsx b/packages/ui/src/features/canvas/components/ChannelSidebar.tsx index cad0552652..0e19184bba 100644 --- a/packages/ui/src/features/canvas/components/ChannelSidebar.tsx +++ b/packages/ui/src/features/canvas/components/ChannelSidebar.tsx @@ -25,6 +25,7 @@ import { Input, MenuLabel, Skeleton, + SkeletonText, } from "@posthog/quill"; import { LOOPS_FLAG } from "@posthog/shared"; import type { TaskRunStatus } from "@posthog/shared/domain-types"; @@ -157,24 +158,30 @@ function RecentSectionHeader({ ); } -// Varied widths so the loading state reads as the list it becomes. -const SKELETON_ROW_WIDTHS = [ - "w-3/5", - "w-4/5", - "w-2/5", - "w-3/4", - "w-1/2", - "w-2/3", -] as const; +// Varied widths, as percentages, so the loading state reads as the list it +// becomes rather than a stack of identical bars. +const SKELETON_ROW_WIDTHS = [60, 80, 40, 75, 50, 66] as const; function ChannelItemsSkeleton() { return (
- + {/* Stands in for the "Recent" MenuLabel, so it carries that label's scale. */} + {SKELETON_ROW_WIDTHS.map((width) => (
- + {/* SkeletonText sizes its bar off the line it stands in — text-[13px] + is a row's own type scale, so the placeholder is the height of the + title it becomes rather than a fixed pill. */} +
))}