From 8142e54150f263815100e52cc3db43b16d630122 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Tue, 4 Aug 2026 23:30:23 -0500 Subject: [PATCH] feat(activity): merge the audit page into one rich Activity page (#10838) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The board has two different pages for change history: a basic Activity list and a rich Audit feed > - The two pages show the same kind of information, so an operator must guess which page to open > - The basic list also caps at 200 rows and has no filters, so it hides older changes > - This pull request merges both pages into one Activity page that is built on the rich audit feed > - The page adds a scope toggle for all actors or agent actions only, and it hides privileged controls from members who do not have the audit permission > - The benefit is one obvious place to answer "who changed what", for every member, with filters and full history ## Linked Issues or Issue Description Related pull requests in this stack (open both before this one): - Refs #10830 — adds the company prefix to the board audit route. - Refs #10831 — adds the two-tier all-actors scope to the audit endpoint. This pull request calls that scope. This branch is stacked on those two pull requests. The diff therefore shows their commits until they merge. After they merge, this pull request contains only the last two commits: the page merge and the actor-label fix. **Problem or motivation** The board has two overlapping history pages. `/:company/activity` renders a plain list that is capped at 200 rows and has no filters. The audit page renders a filtered, paginated feed of agent actions, but it is a separate sidebar item and it was reachable only by members with the audit permission. A member who wants to know who changed an issue must know which of the two pages answers the question. **Proposed solution** Keep one sidebar item, "Activity", and build it on the rich feed. Add a scope toggle: "All activity" reads every actor kind, and "Agent actions" keeps the earlier audit behavior. Put the scope in the `mode` query parameter so a person can link to it. Show the responsible-user filter and the CSV export only to callers that the server answers at the privileged tier. Redirect the earlier audit paths to the merged page with the agent scope preset, so old links continue to work. Delete the plain list page. **Alternatives considered** Keeping both pages and adding filters to the plain list. That duplicates the feed logic and keeps the "which page?" problem. Deleting the audit page instead was also rejected, because the audit feed has the pagination, filters, and export that the plain list does not. **Roadmap alignment** The roadmap marks the activity log and action attribution as shipped. This change improves that shipped capability. It does not add a new subsystem. ## What Changed - Added a scope toggle to `AuditFeed`. "All activity" requests `actorScope=all`, and "Agent actions" keeps the earlier agent-only request. Cursor pagination works in both scopes. - Stored the scope in the `mode` query parameter, so a person can bookmark or share a scope. - Made the page chrome permission-aware. The toggle, the responsible-user filter, and the CSV export appear only when the server answers at the privileged tier. A basic member sees the shared feed and no upsell wall. - Replaced the sidebar "Audit" item. The sidebar now has one "Activity" item. - Redirected `/:company/audit` and the unprefixed `/audit` to `/:company/activity?mode=agents`. - Deleted the earlier `ui/src/pages/Activity.tsx` list page and the `CompanyAudit` page wrapper. Added `CompanyActivity` as the single route target. - Fixed the actor label for stripped rows. The basic tier removes the agent id but keeps the actor kind, so every agent row rendered as "System". Rows now fall back to the actor kind: "Agent", "User", "Plugin", or "System". - Widened the responsible-user filter control, which truncated its own label. - Resolved agent names on the basic tier. The basic tier removes the privileged `agentId` but keeps the acting principal `actorId`, and the company agent directory this page already reads is authorization-filtered. The feed therefore resolves an agent actor from `agentId` first and from an agent-typed `actorId` second. Hiding the name only in the UI gave no confidentiality benefit, because any reader could join the retained id against the readable directory. Agents that the directory filters out still fall back to the generic kind label. No server payload or permission was widened. - Fixed a stuck state in the access-downgrade recovery. A downgrade between cursor requests leaves full-tier and basic-tier pages in one cache, which starts a single recovery refetch. If that refetch did not clear the mix, the cached pages kept the condition true, the "Refreshing audit access…" banner rendered permanently, and it hid the error state together with its "Try again" button. The banner is now tied to an outstanding attempt. The refetch effect also depended on the whole query object, which changes identity every render, so it repeated the request on each render; the attempt is now tracked in state and runs once per downgrade. - Kept the agent detail "Audit" tab unchanged. That tab passes a locked agent id, which keeps the earlier privileged scope and hides the toggle. The `GET /companies/:id/activity` endpoint stays. The dashboard still reads it. This pull request does not change that endpoint. ## Verification - `pnpm exec vitest run ui/src/pages/audit/AuditFeed.test.tsx ui/src/App.activity-routing.test.tsx ui/src/lib/company-routes.test.ts ui/src/components/Sidebar.test.tsx server/src/__tests__/activity-routes.test.ts server/src/__tests__/agent-action-audit-routes.test.ts` — all tests pass. - New `ui/src/App.activity-routing.test.tsx` drives the real route table. It asserts that the company activity path resolves, and that both the company audit path and the unprefixed audit path reach the activity path with the agent scope preset. - New `AuditFeed` tests cover the scope toggle, the basic tier without privileged chrome, the locked-agent case, the actor-kind fallback label, basic-tier name resolution, and both downgrade-recovery paths (the refetch errors, and the refetch returns a still-mixed pair). - Mutation-checked the three new guards: disabling each one fails the test that covers it, so none of them pass vacuously. - `pnpm -r typecheck` is clean. Both design token gates are clean. - Rendered every state in a browser at 1440x900 and at 390x844: both scopes, the basic member view, the loading state, the error state, the filtered-empty state, and the true-empty state. A designer reviewed the renders and approved them. ## Risks - The default company page now reads the all-actors scope, which returns more rows than the earlier agent-only query. Cursor pagination and the existing page limit bound each request. - The page is now visible to every company member. The server decides what each member sees. The UI only hides controls that the caller cannot use. Refs #10831 for the server rules and tests. - The basic tier now shows agent names that the previous revision withheld. The name was already recoverable from the retained `actorId` through the readable agent directory, so this closes an inconsistency rather than widening access. A security reviewer chose this outcome over stripping `actorId`. - Old audit links now redirect. The redirect keeps the agent scope, so a person who bookmarked the audit page sees the same rows. - Low migration risk. There is no database change. > The roadmap marks activity log and action attribution as shipped. This change improves that existing capability. ## Model Used Claude Opus 5 (`claude-opus-5`, 1M context) with extended thinking and tool use, run through Claude Code. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip Co-authored-by: Claude Opus 5 (1M context) --- ui/src/App.activity-routing.test.tsx | 171 ++++++++++++++++++++ ui/src/App.tsx | 9 +- ui/src/components/Sidebar.tsx | 3 +- ui/src/pages/Activity.tsx | 161 ------------------- ui/src/pages/audit/AuditFeed.test.tsx | 214 ++++++++++++++++++++++++- ui/src/pages/audit/AuditFeed.tsx | 144 ++++++++++++++--- ui/src/pages/audit/CompanyActivity.tsx | 49 ++++++ ui/src/pages/audit/CompanyAudit.tsx | 25 --- 8 files changed, 558 insertions(+), 218 deletions(-) create mode 100644 ui/src/App.activity-routing.test.tsx delete mode 100644 ui/src/pages/Activity.tsx create mode 100644 ui/src/pages/audit/CompanyActivity.tsx delete mode 100644 ui/src/pages/audit/CompanyAudit.tsx diff --git a/ui/src/App.activity-routing.test.tsx b/ui/src/App.activity-routing.test.tsx new file mode 100644 index 0000000000..534bf3ecb1 --- /dev/null +++ b/ui/src/App.activity-routing.test.tsx @@ -0,0 +1,171 @@ +// @vitest-environment jsdom + +// Regression guard for PAP-16302: `/audit` was merged into the single rich +// Activity page. Both the company-prefixed `/:company/audit` and the bare +// `/audit` (PAP-16300's unprefixed redirect) must keep resolving — as redirects +// into `/:company/activity?mode=agents`, so old deep links land on the +// agent-actions scope instead of 404ing. This drives the real route table +// so removing either registration fails loudly. + +import type { ReactNode } from "react"; +import { flushSync } from "react-dom"; +import { createRoot } from "react-dom/client"; +import { MemoryRouter, useLocation } from "react-router-dom"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { App } from "./App"; + +// jsdom's CSS parser rejects the custom-property marker rule stitches inserts +// (`--sxs{--sxs:N}`), pulled into 's eager import graph transitively via +// @codesandbox/sandpack-react. Substitute a benign, valid rule on parse failure +// so stitches' index bookkeeping stays intact and the module graph evaluates. +vi.hoisted(() => { + const sheetProto = window.CSSStyleSheet.prototype as unknown as { + insertRule: (rule: string, index?: number) => number; + __papActivityRoutingPatched?: boolean; + }; + if (!sheetProto.__papActivityRoutingPatched) { + const original = sheetProto.insertRule; + sheetProto.insertRule = function patched(this: CSSStyleSheet, rule: string, index?: number) { + try { + return original.call(this, rule, index); + } catch { + try { + return original.call(this, ".pap16302-noop{}", index); + } catch { + return this.cssRules?.length ?? 0; + } + } + }; + sheetProto.__papActivityRoutingPatched = true; + } +}); + +// Real Layout renders the full authenticated shell (sidebar, data queries) and +// owns the "No company matches prefix" NotFound. For routing we only need it to +// resolve the :companyPrefix segment and render its nested routes. +vi.mock("./components/Layout", async () => { + const { Outlet } = await import("react-router-dom"); + return { Layout: () => }; +}); + +// Rendered by outside and needs DialogProvider; irrelevant here. +vi.mock("./components/OnboardingWizardVariant", () => ({ + OnboardingWizardVariant: () => null, +})); + +// Cloud access is unrelated to the route-table regression. Let it fall through +// synchronously so this test does not poll its query transitions. +vi.mock("./components/CloudAccessGate", async () => { + const { Outlet } = await import("react-router-dom"); + return { CloudAccessGate: () => }; +}); + +// Sentinel page that also reports the resolved path + query, so we can assert +// the merged route *and* the preset mode a redirect carried into it. +vi.mock("./pages/audit/CompanyActivity", () => ({ + CompanyActivity: () => { + const location = useLocation(); + return
{`ACTIVITY_PAGE@${location.pathname}${location.search}`}
; + }, +})); + +const PAP_COMPANY = { + id: "company-1", + name: "Paperclip", + issuePrefix: "PAP", + status: "active", +}; +const ACME_COMPANY = { + id: "company-2", + name: "Acme", + issuePrefix: "ACME", + status: "active", +}; + +// Mutable so a test can put the *selected* company out of step with the company +// in the URL — that mismatch is what catches a redirect that re-resolves the +// company from context instead of keeping the one the deep link named. +let companyState = { + companies: [PAP_COMPANY] as Array, + selected: PAP_COMPANY as typeof PAP_COMPANY | null, +}; +vi.mock("./context/CompanyContext", () => ({ + useCompany: () => ({ + companies: companyState.companies, + selectedCompanyId: companyState.selected?.id ?? null, + selectedCompany: companyState.selected, + loading: false, + }), + CompanyProvider: ({ children }: { children: ReactNode }) => <>{children}, +})); + +function renderAppAt(container: HTMLElement, path: string) { + const root = createRoot(container); + flushSync(() => { + root.render( + + + , + ); + }); + return root; +} + +async function waitForRoute(container: HTMLElement, text: string) { + for (let attempt = 0; attempt < 5; attempt += 1) { + if (container.textContent?.includes(text)) return; + await new Promise((resolve) => window.setTimeout(resolve, 0)); + } + expect(container.textContent).toContain(text); +} + +describe("App Activity routing (PAP-16302)", () => { + let container: HTMLDivElement; + + beforeEach(() => { + container = document.createElement("div"); + document.body.appendChild(container); + companyState = { companies: [PAP_COMPANY], selected: PAP_COMPANY }; + }); + + afterEach(() => { + container.remove(); + document.body.innerHTML = ""; + vi.clearAllMocks(); + }); + + it("serves the merged Activity page at /:company/activity", async () => { + const root = renderAppAt(container, "/PAP/activity"); + await waitForRoute(container, "ACTIVITY_PAGE@/PAP/activity"); + expect(container.textContent).not.toContain("No company matches prefix"); + flushSync(() => root.unmount()); + }); + + it("redirects /:company/audit to Activity with the agent-actions mode preset", async () => { + const root = renderAppAt(container, "/PAP/audit"); + await waitForRoute(container, "ACTIVITY_PAGE@/PAP/activity?mode=agents"); + flushSync(() => root.unmount()); + }); + + it("keeps the company from the URL when /:company/audit is not the selected company", async () => { + // A shared /ACME/audit link opened by someone whose selected company is PAP + // must still show ACME's activity. The redirect target is written absolute + // (`/activity?mode=agents`) and relies on `@/lib/router`'s prefix-aware + // , which resolves the company from the route param ahead of the + // selected company. Importing from `react-router-dom` instead — + // the import most files use — would send this deep link to the *viewer's* + // company, so pin the behaviour here. + companyState = { companies: [PAP_COMPANY, ACME_COMPANY], selected: PAP_COMPANY }; + const root = renderAppAt(container, "/ACME/audit"); + await waitForRoute(container, "ACTIVITY_PAGE@/ACME/activity?mode=agents"); + expect(container.textContent).not.toContain("ACTIVITY_PAGE@/PAP/activity"); + flushSync(() => root.unmount()); + }); + + it("redirects the bare /audit deep link through to the prefixed Activity page", async () => { + const root = renderAppAt(container, "/audit"); + await waitForRoute(container, "ACTIVITY_PAGE@/PAP/activity?mode=agents"); + expect(container.textContent).not.toContain("No company matches prefix"); + flushSync(() => root.unmount()); + }); +}); diff --git a/ui/src/App.tsx b/ui/src/App.tsx index e862c3422f..f772a71251 100644 --- a/ui/src/App.tsx +++ b/ui/src/App.tsx @@ -41,8 +41,7 @@ import { GoalDetail } from "./pages/GoalDetail"; import { Approvals } from "./pages/Approvals"; import { ApprovalDetail } from "./pages/ApprovalDetail"; import { Costs } from "./pages/Costs"; -import { Activity } from "./pages/Activity"; -import { CompanyAudit } from "./pages/audit/CompanyAudit"; +import { CompanyActivity } from "./pages/audit/CompanyActivity"; import { Inbox } from "./pages/Inbox"; import { WhatNeedsMe } from "./pages/WhatNeedsMe"; import { DecisionQueuePage } from "./pages/DecisionQueuePage"; @@ -262,8 +261,10 @@ function boardRoutes() { } /> } /> } /> - } /> - } /> + } /> + {/* `/audit` merged into the single Activity page (PAP-16302). Existing deep + links keep working, preset to the agent-actions scope. */} + } /> {/* Conference Room Chat surfaces (PAP-136/PAP-137): routes stay registered but redirect to the company home while the experimental flag is off. The board-level `artifacts` mount below is the new diff --git a/ui/src/components/Sidebar.tsx b/ui/src/components/Sidebar.tsx index 67d2152e1f..c40d0b27af 100644 --- a/ui/src/components/Sidebar.tsx +++ b/ui/src/components/Sidebar.tsx @@ -22,7 +22,6 @@ import { AppWindow, MessagesSquare, GanttChartSquare, - ScrollText, LayoutGrid, } from "lucide-react"; import { useState } from "react"; @@ -279,8 +278,8 @@ export function Sidebar() { {showApps ? : null} + {/* One entry — /audit merged into the rich Activity feed (PAP-16302). */} - diff --git a/ui/src/pages/Activity.tsx b/ui/src/pages/Activity.tsx deleted file mode 100644 index b335b4458b..0000000000 --- a/ui/src/pages/Activity.tsx +++ /dev/null @@ -1,161 +0,0 @@ -import { useEffect, useMemo, useState } from "react"; -import { useQuery } from "@tanstack/react-query"; -import type { ActivityEvent, Agent } from "@paperclipai/shared"; -import { activityApi } from "../api/activity"; -import { accessApi } from "../api/access"; -import { agentsApi } from "../api/agents"; -import { buildCompanyUserProfileMap } from "../lib/company-members"; -import { useCompany } from "../context/CompanyContext"; -import { useBreadcrumbs } from "../context/BreadcrumbContext"; -import { queryKeys } from "../lib/queryKeys"; -import { EmptyState } from "../components/EmptyState"; -import { ActivityRow } from "../components/ActivityRow"; -import { PageSkeleton } from "../components/PageSkeleton"; -import { - Select, - SelectContent, - SelectItem, - SelectTrigger, - SelectValue, -} from "@/components/ui/select"; -import { History } from "lucide-react"; -import { Card } from "@/components/ui/card"; - -const ACTIVITY_PAGE_LIMIT = 200; - -function detailString(event: ActivityEvent, ...keys: string[]) { - const details = event.details; - for (const key of keys) { - const value = details?.[key]; - if (typeof value === "string" && value.trim()) return value; - } - return null; -} - -function activityEntityName(event: ActivityEvent) { - if (event.entityType === "issue") return detailString(event, "identifier", "issueIdentifier"); - if (event.entityType === "project") return detailString(event, "projectName", "name", "title"); - if (event.entityType === "goal") return detailString(event, "goalTitle", "title", "name"); - return detailString(event, "name", "title"); -} - -function activityEntityTitle(event: ActivityEvent) { - if (event.entityType === "issue") return detailString(event, "issueTitle", "title"); - return null; -} - -export function Activity() { - const { selectedCompanyId } = useCompany(); - const { setBreadcrumbs } = useBreadcrumbs(); - const [filter, setFilter] = useState("all"); - - useEffect(() => { - setBreadcrumbs([{ label: "Activity" }]); - }, [setBreadcrumbs]); - - const { data, isLoading, error } = useQuery({ - queryKey: [...queryKeys.activity(selectedCompanyId!), { limit: ACTIVITY_PAGE_LIMIT }], - queryFn: () => activityApi.list(selectedCompanyId!, { limit: ACTIVITY_PAGE_LIMIT }), - enabled: !!selectedCompanyId, - }); - - const { data: agents } = useQuery({ - queryKey: queryKeys.agents.list(selectedCompanyId!), - queryFn: () => agentsApi.list(selectedCompanyId!), - enabled: !!selectedCompanyId, - }); - - const { data: companyMembers } = useQuery({ - queryKey: queryKeys.access.companyUserDirectory(selectedCompanyId!), - queryFn: () => accessApi.listUserDirectory(selectedCompanyId!), - enabled: !!selectedCompanyId, - }); - - const userProfileMap = useMemo( - () => buildCompanyUserProfileMap(companyMembers?.users), - [companyMembers?.users], - ); - - const agentMap = useMemo(() => { - const map = new Map(); - for (const a of agents ?? []) map.set(a.id, a); - return map; - }, [agents]); - - const entityNameMap = useMemo(() => { - const map = new Map(); - for (const a of agents ?? []) map.set(`agent:${a.id}`, a.name); - for (const event of data ?? []) { - const name = activityEntityName(event); - if (name) map.set(`${event.entityType}:${event.entityId}`, name); - } - return map; - }, [data, agents]); - - const entityTitleMap = useMemo(() => { - const map = new Map(); - for (const event of data ?? []) { - const title = activityEntityTitle(event); - if (title) map.set(`${event.entityType}:${event.entityId}`, title); - } - return map; - }, [data]); - - if (!selectedCompanyId) { - return ; - } - - if (isLoading) { - return ; - } - - const filtered = - data && filter !== "all" - ? data.filter((e) => e.entityType === filter) - : data; - - const entityTypes = data - ? [...new Set(data.map((e) => e.entityType))].sort() - : []; - - return ( -
-
- -
- - {error &&

{error.message}

} - - {filtered && filtered.length === 0 && ( - - )} - - {filtered && filtered.length > 0 && ( - - {filtered.map((event) => ( - - ))} - - )} -
- ); -} diff --git a/ui/src/pages/audit/AuditFeed.test.tsx b/ui/src/pages/audit/AuditFeed.test.tsx index 32374f5840..5b4f4bb661 100644 --- a/ui/src/pages/audit/AuditFeed.test.tsx +++ b/ui/src/pages/audit/AuditFeed.test.tsx @@ -1,7 +1,7 @@ // @vitest-environment jsdom import { flushSync } from "react-dom"; -import type { ReactNode } from "react"; +import { useState, type ReactNode } from "react"; import { createRoot } from "react-dom/client"; import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; @@ -105,13 +105,25 @@ describe("AuditFeed", () => { vi.clearAllMocks(); }); - async function render(props: { companyId?: string; lockedAgentId?: string } = {}) { + async function render( + props: { + companyId?: string; + lockedAgentId?: string; + mode?: "all" | "agents"; + onModeChange?: (mode: "all" | "agents") => void; + } = {}, + ) { const client = new QueryClient({ defaultOptions: { queries: { retry: false } } }); root = createRoot(container); await act(async () => { root.render( - + , ); }); @@ -119,6 +131,18 @@ describe("AuditFeed", () => { return client; } + /** Poll until `text` renders, for states that settle behind query retry backoff. */ + async function waitForText(text: string, timeoutMs: number) { + const deadline = Date.now() + timeoutMs; + while (Date.now() < deadline) { + if (container.textContent?.includes(text)) return; + await act(async () => { + await new Promise((resolve) => window.setTimeout(resolve, 25)); + }); + } + expect(container.textContent, `waiting for "${text}"`).toContain(text); + } + function clickButton(text: string) { const btn = Array.from(container.querySelectorAll("button")).find((b) => b.textContent?.includes(text)); expect(btn, `button "${text}"`).toBeTruthy(); @@ -127,6 +151,18 @@ describe("AuditFeed", () => { }); } + /** Radix tabs activate on mousedown, so drive both events like a real click. */ + function clickTab(label: string) { + const tab = Array.from(container.querySelectorAll('[role="tab"]')).find( + (el) => el.textContent?.trim() === label, + ); + expect(tab, `tab "${label}"`).toBeTruthy(); + return act(async () => { + tab!.dispatchEvent(new MouseEvent("mousedown", { bubbles: true, button: 0 })); + tab!.dispatchEvent(new MouseEvent("click", { bubbles: true, button: 0 })); + }); + } + it("renders the humanized sentence, the task link, the excerpt, and the on-behalf chip", async () => { await render(); @@ -249,6 +285,75 @@ describe("AuditFeed", () => { expect(container.textContent).not.toContain("Export CSV"); }); + it("surfaces the retry UI when the access-downgrade recovery refetch fails", async () => { + // The downgrade recovery refetch can itself fail. React Query keeps the + // cached mixed-tier pages on failure, so without a guard the feed sits on + // "Refreshing audit access…" forever with no way out. + let downgraded = false; + let calls = 0; + listAgentActionsMock.mockImplementation((_companyId: string, filters: { cursor?: string }) => { + calls += 1; + if (filters.cursor === "cursor-2") { + downgraded = true; + return Promise.resolve({ + items: [record({ id: "evt-2", agentId: null, runId: null, responsibleUserId: null, details: null })], + nextCursor: null, + accessTier: "basic", + }); + } + if (downgraded) return Promise.reject(new Error("Network down")); + return Promise.resolve({ items: [record()], nextCursor: "cursor-2", accessTier: "full" }); + }); + await render(); + await clickButton("Load more"); + // The feed retries twice with backoff (~3s) before the query settles as + // errored, and it stays "fetching" throughout — so wait for the settled + // state rather than a fixed number of microtask flushes. The explicit test + // timeout below keeps that wait inside the budget on slower CI runners. + await waitForText("Try again", 20_000); + + expect(container.textContent).not.toContain("Refreshing audit access…"); + expect(container.textContent).toContain("Network down"); + expect(container.textContent).toContain("Try again"); + + // And the recovery must not loop: no further refetches once it has failed. + const settledCalls = calls; + await flushReact(); + expect(calls).toBe(settledCalls); + }, 30_000); + + it("renders the feed when the recovery refetch cannot clear the mixed tiers", async () => { + // Pathological but reachable: the refetch succeeds and still returns one + // full page plus one basic page. The recovery has had its shot, so the feed + // must render at the least-privileged tier instead of sitting on the banner. + listAgentActionsMock.mockImplementation((_companyId: string, filters: { cursor?: string }) => + filters.cursor === "cursor-2" + ? Promise.resolve({ + items: [record({ id: "evt-2", agentId: null, runId: null, responsibleUserId: null, details: null })], + nextCursor: null, + accessTier: "basic", + }) + : Promise.resolve({ items: [record()], nextCursor: "cursor-2", accessTier: "full" }), + ); + await render(); + await clickButton("Load more"); + await flushReact(); + await flushReact(); + + expect(container.textContent).not.toContain("Refreshing audit access…"); + expect(container.textContent).toContain("commented on"); + // Least-privileged page wins, so the privileged chrome stays hidden. + expect(container.textContent).not.toContain("Export CSV"); + // And the cached full-tier page must not render revoked attribution beside + // the stripped rows just because the recovery has run out of attempts. + expect(container.textContent).not.toContain("on behalf of Dotta"); + expect(container.querySelector('a[href="/agents/agent-1/runs/run-1"]')).toBeFalsy(); + + const settledCalls = listAgentActionsMock.mock.calls.length; + await flushReact(); + expect(listAgentActionsMock.mock.calls.length).toBe(settledCalls); + }); + it("hides the agent filter and pins the query when lockedAgentId is set", async () => { await render({ lockedAgentId: "agent-1" }); @@ -258,6 +363,109 @@ describe("AuditFeed", () => { expect(container.textContent).not.toContain("All agents"); }); + it("ignores the mode toggle on the per-agent tab", async () => { + const onModeChange = vi.fn(); + await render({ lockedAgentId: "agent-1", mode: "all", onModeChange }); + + expect(container.querySelector('[role="tab"]')).toBeFalsy(); + expect(listAgentActionsMock.mock.calls[0]?.[1]).toEqual( + expect.objectContaining({ actorScope: "agents", agentId: "agent-1" }), + ); + expect(onModeChange).not.toHaveBeenCalled(); + }); + + it("offers the agent-actions mode to a full-tier reader and requests the privileged scope", async () => { + // Mirror the page: the mode lives above the feed, so toggling re-queries. + function Harness() { + const [mode, setMode] = useState<"all" | "agents">("all"); + return ; + } + const client = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + root = createRoot(container); + await act(async () => { + root.render( + + + , + ); + }); + await flushReact(); + + expect(container.textContent).toContain("All activity"); + expect(container.textContent).toContain("Agent actions"); + expect(listAgentActionsMock.mock.calls[0]?.[1]).toEqual( + expect.objectContaining({ actorScope: "all" }), + ); + + await clickTab("Agent actions"); + await flushReact(); + + expect(listAgentActionsMock.mock.calls.at(-1)?.[1]).toEqual( + expect.objectContaining({ actorScope: "agents" }), + ); + // The privileged scope keeps the attribution filters and the export. + expect(container.textContent).toContain("All responsible users"); + expect(container.textContent).toContain("Export CSV"); + }); + + it("resolves a basic-tier agent name from the company-readable actorId", async () => { + // The basic tier nulls privileged attribution but retains the acting + // principal, which is also available through the company agent directory. + listAgentActionsMock.mockResolvedValue({ + items: [record({ agentId: null, runId: null, responsibleUserId: null, details: null })], + nextCursor: null, + accessTier: "basic", + }); + await render({ mode: "all", onModeChange: vi.fn() }); + + expect(container.textContent).toContain("Fable"); + expect(container.textContent).not.toContain("Agent commented"); + expect(container.textContent).not.toContain("System"); + }); + + it("falls back to the actor type when an agent is absent from the readable directory", async () => { + listAgentActionsMock.mockResolvedValue({ + items: [record({ actorId: "filtered-agent", agentId: null, runId: null, responsibleUserId: null, details: null })], + nextCursor: null, + accessTier: "basic", + }); + await render({ mode: "all", onModeChange: vi.fn() }); + + expect(container.textContent).toContain("Agent"); + expect(container.textContent).not.toContain("System"); + }); + + it("hides the mode toggle from a basic all-actors reader", async () => { + listAgentActionsMock.mockResolvedValue({ items: [record()], nextCursor: null, accessTier: "basic" }); + await render({ mode: "all", onModeChange: vi.fn() }); + + expect(container.querySelector('[role="tab"]')).toBeFalsy(); + expect(container.textContent).not.toContain("Agent actions"); + // The basic feed itself still renders. + expect(container.textContent).toContain("commented on"); + }); + + it("falls back to all-activity instead of the upsell when a basic reader opens the agent-actions mode", async () => { + listAgentActionsMock.mockRejectedValue( + new ApiError("Missing permission: audit:view_agent_actions", 403, { error: "Missing permission" }), + ); + const onModeChange = vi.fn(); + await render({ mode: "agents", onModeChange }); + + expect(onModeChange).toHaveBeenCalledWith("all"); + expect(container.textContent).not.toContain("Paperclip Enterprise view"); + expect(container.textContent).toContain("Refreshing audit access…"); + }); + + it("still upsells an uncontrolled agent-actions feed that 403s", async () => { + listAgentActionsMock.mockRejectedValue( + new ApiError("Missing permission: audit:view_agent_actions", 403, { error: "Missing permission" }), + ); + await render({ mode: "agents" }); + + expect(container.textContent).toContain("Paperclip Enterprise view"); + }); + it("only offers action domains present in the agent-action feed", async () => { await render(); diff --git a/ui/src/pages/audit/AuditFeed.tsx b/ui/src/pages/audit/AuditFeed.tsx index 740027519c..94426bfcda 100644 --- a/ui/src/pages/audit/AuditFeed.tsx +++ b/ui/src/pages/audit/AuditFeed.tsx @@ -13,6 +13,7 @@ import { SelectTrigger, SelectValue, } from "@/components/ui/select"; +import { Tabs, TabsList, TabsTrigger } from "@/components/ui/tabs"; import { Identity } from "@/components/Identity"; import { AgentIcon } from "@/components/AgentIconPicker"; import { cn, relativeTime } from "@/lib/utils"; @@ -52,6 +53,13 @@ const ENTITY_TYPES: { value: string; label: string }[] = [ { value: "company", label: "Company" }, ]; +/** + * Which actors the feed covers. `all` is the shared company activity view + * (people, agents, and the system); `agents` is the privileged agent-action + * audit that carries responsible-person and run attribution. + */ +export type AuditFeedMode = "all" | "agents"; + export interface AuditFeedProps { companyId: string; /** @@ -61,6 +69,13 @@ export interface AuditFeedProps { lockedAgentId?: string; /** Hide the section header/description (the AgentDetail tab supplies its own chrome). */ hideHeader?: boolean; + /** + * Controlled feed mode. Supplying `onModeChange` turns on the mode toggle for + * callers that hold `audit:view_agent_actions`; without it the feed stays in + * `mode` (or the all-actors default). Ignored when `lockedAgentId` is set. + */ + mode?: AuditFeedMode; + onModeChange?: (mode: AuditFeedMode) => void; } function toStartIso(value: string): string | undefined { @@ -85,7 +100,14 @@ function AuditActor({ agentMap: Map; userProfileMap: Map; }) { - const agent = record.agentId ? agentMap.get(record.agentId) : null; + // Agent names are company-readable through the same authorization-filtered + // directory used by this page. The basic audit tier strips privileged + // attribution (`agentId`) but retains the acting principal (`actorId`), so + // use that principal to avoid presenting a trivially joinable identity as + // an anonymous "Agent" in the UI. + const actorAgentId = record.agentId + ?? (record.actorType === "agent" ? record.actorId : null); + const agent = actorAgentId ? agentMap.get(actorAgentId) : null; if (agent) { return ( @@ -107,7 +129,16 @@ function AuditActor({ /> ); } - const label = record.actorType === "plugin" ? "Plugin" : "System"; + // Fall back to the actor *type*, never a blanket "System". This still covers + // deleted or authorization-filtered agents that are absent from the directory. + const label = + record.actorType === "plugin" + ? "Plugin" + : record.actorType === "agent" + ? "Agent" + : record.actorType === "user" + ? "User" + : "System"; return ; } @@ -226,7 +257,13 @@ function AuditUpsell() { ); } -export function AuditFeed({ companyId, lockedAgentId, hideHeader }: AuditFeedProps) { +export function AuditFeed({ + companyId, + lockedAgentId, + hideHeader, + mode, + onModeChange, +}: AuditFeedProps) { const { pushToast } = useToastActions(); const [agent, setAgent] = useState(ALL); const [responsibleUser, setResponsibleUser] = useState(ALL); @@ -235,6 +272,7 @@ export function AuditFeed({ companyId, lockedAgentId, hideHeader }: AuditFeedPro const [dateFrom, setDateFrom] = useState(""); const [dateTo, setDateTo] = useState(""); const [exporting, setExporting] = useState(false); + const [downgradeRecoveryAttempted, setDowngradeRecoveryAttempted] = useState(false); const agents = useQuery({ queryKey: queryKeys.agents.list(companyId), @@ -255,11 +293,13 @@ export function AuditFeed({ companyId, lockedAgentId, hideHeader }: AuditFeedPro [userDirectory.data], ); + // The per-agent tab keeps the legacy privileged scope because it always + // carries an attribution filter and must not silently downgrade to the basic + // tier. Everywhere else the mode picks the scope, defaulting to all actors. + const resolvedMode: AuditFeedMode = lockedAgentId ? "agents" : mode ?? "all"; + const filters: AuditActionFilters = { - // The company feed is the shared all-actors view. The per-agent tab keeps - // the legacy privileged scope because it always carries an attribution - // filter and must not silently downgrade to the basic tier. - actorScope: lockedAgentId ? "agents" : "all", + actorScope: resolvedMode, agentId: lockedAgentId ?? (agent === ALL ? undefined : agent), responsibleUserId: responsibleUser === ALL ? undefined : responsibleUser, action: actionDomain === ALL ? undefined : actionDomain, @@ -294,14 +334,19 @@ export function AuditFeed({ companyId, lockedAgentId, hideHeader }: AuditFeedPro retry: (count, error) => !(error instanceof ApiError && error.status === 403) && count < 2, }); - const items = useMemo( - () => feed.data?.pages.flatMap((page) => page.items) ?? [], - [feed.data], - ); - const permissionDenied = feed.error instanceof ApiError && feed.error.status === 403; const hasBasicPage = feed.data?.pages.some((page) => page.accessTier === "basic") ?? false; const hasFullPage = feed.data?.pages.some((page) => page.accessTier === "full") ?? false; + + // Once the server answers at the basic tier the caller has lost the permission + // that produced the privileged attribution on the pages already in the cache. + // Drop those pages rather than rendering revoked "on behalf of" attribution + // next to stripped rows — the recovery refetch below may never clear them. + const items = useMemo(() => { + const pages = feed.data?.pages ?? []; + const visible = hasBasicPage ? pages.filter((page) => page.accessTier !== "full") : pages; + return visible.flatMap((page) => page.items); + }, [feed.data, hasBasicPage]); // Access may be revoked between cursor requests. Treat the least-privileged // page as authoritative until every cached page has been fetched again. const accessTier = hasBasicPage ? "basic" : feed.data?.pages[0]?.accessTier; @@ -309,10 +354,40 @@ export function AuditFeed({ companyId, lockedAgentId, hideHeader }: AuditFeedPro const canUseAdvancedControls = lockedAgentId ? true : accessTier === "full"; + // The recovery refetch below gets one shot. If it does not clear the mixed + // pages — it errored, or it somehow came back mixed again — the cache keeps + // them, so `hasMixedAccessTiers` would stay true forever. Only call the feed + // "recovering" while that attempt is outstanding; once it has settled, fall + // through to normal rendering. Otherwise the banner permanently hides the + // error state and its "Try again" button, with no way off the page. Falling + // through is safe because `items` already excludes the privileged pages, so + // an unrecovered cache renders as a plain basic-tier feed. + const downgradeRecoveryExhausted = Boolean( + hasMixedAccessTiers && downgradeRecoveryAttempted && !feed.isFetching, + ); const recoveringFromAccessDowngrade = Boolean( !lockedAgentId + && !downgradeRecoveryExhausted && ((permissionDenied && hasActiveFilters) || hasMixedAccessTiers), ); + // A reader without `audit:view_agent_actions` can still land on the + // agent-actions mode through an old `/audit` deep link. Drop them into the + // shared all-activity feed instead of blocking the whole page with the upsell. + const fallingBackToAllActivity = Boolean( + permissionDenied && !lockedAgentId && resolvedMode === "agents" && onModeChange, + ); + // The privileged mode is only offered to callers the server already answered + // at the full tier — everyone else just gets the basic all-activity feed. + const showModeToggle = Boolean( + !lockedAgentId + && onModeChange + && !fallingBackToAllActivity + && (resolvedMode === "agents" || accessTier === "full"), + ); + + useEffect(() => { + if (fallingBackToAllActivity) onModeChange?.("all"); + }, [fallingBackToAllActivity, onModeChange]); useEffect(() => { if (!lockedAgentId && (accessTier === "basic" || recoveringFromAccessDowngrade)) { @@ -323,10 +398,20 @@ export function AuditFeed({ companyId, lockedAgentId, hideHeader }: AuditFeedPro setDateFrom(""); setDateTo(""); } - if (hasMixedAccessTiers) { - void feed.refetch(); + }, [accessTier, hasMixedAccessTiers, lockedAgentId, recoveringFromAccessDowngrade]); + + // Recover from a mid-pagination downgrade with exactly one refetch. `feed` + // gets a new identity on every render, so an unguarded refetch here re-fires + // on each render and hammers the endpoint while the tiers stay mixed. + useEffect(() => { + if (!hasMixedAccessTiers) { + if (downgradeRecoveryAttempted) setDowngradeRecoveryAttempted(false); + return; } - }, [accessTier, feed, hasMixedAccessTiers, lockedAgentId, recoveringFromAccessDowngrade]); + if (downgradeRecoveryAttempted) return; + setDowngradeRecoveryAttempted(true); + void feed.refetch(); + }, [downgradeRecoveryAttempted, feed, hasMixedAccessTiers]); const clearFilters = () => { setAgent(ALL); @@ -352,7 +437,7 @@ export function AuditFeed({ companyId, lockedAgentId, hideHeader }: AuditFeedPro const url = URL.createObjectURL(blob); const link = document.createElement("a"); link.href = url; - link.download = `agent-audit-${companyId}.csv`; + link.download = `${resolvedMode === "agents" ? "agent-audit" : "activity"}-${companyId}.csv`; document.body.appendChild(link); link.click(); link.remove(); @@ -371,7 +456,7 @@ export function AuditFeed({ companyId, lockedAgentId, hideHeader }: AuditFeedPro } }; - if (permissionDenied && !recoveringFromAccessDowngrade) { + if (permissionDenied && !recoveringFromAccessDowngrade && !fallingBackToAllActivity) { return ; } @@ -380,15 +465,25 @@ export function AuditFeed({ companyId, lockedAgentId, hideHeader }: AuditFeedPro {!hideHeader ? (
-

Audit

+

Activity

- Everything your company did, newest first — each line is one recorded action. Full - audit access also shows responsible-person and run attribution. + {resolvedMode === "agents" + ? "Every recorded agent action, newest first — with the responsible person and run behind each one." + : "Everything happening in your company, newest first — people, agents, and the system. Each line is one recorded action."}

) : null} + {showModeToggle ? ( + onModeChange?.(value as AuditFeedMode)}> + + All activity + Agent actions + + + ) : null} + {canUseAdvancedControls ? (
{!lockedAgentId ? ( @@ -407,7 +502,8 @@ export function AuditFeed({ companyId, lockedAgentId, hideHeader }: AuditFeedPro ) : null}