diff --git a/doc/DEVELOPING.md b/doc/DEVELOPING.md index 50f62d5faf7..816536af5ca 100644 --- a/doc/DEVELOPING.md +++ b/doc/DEVELOPING.md @@ -5197,6 +5197,14 @@ The browser saves a small read cache in `paperclip:query-cache:v1:*` local stora Restored identity permits cached display only. Restored inline task drafts compare their saved base with a fresh field read before each replay; conflicting edits stay in the editor for review. Account-bound writes wait for a fresh server session and are cancelled if its user changes. A definitive session denial removes saved account reads. Service-worker updates reload hidden tabs only after connectivity is online and mutations, durable sends and autosaves have settled. Durable queues synchronously report each unresolved operation, including the intervals between online rate-limit retries. Sign-out revocation also works on plain-HTTP installs without `crypto.randomUUID`. +The read cache also retains up to 100 learned link previews for the current user +and accessible companies, including unavailable results. Reloading restores the +last known PR status without a provider request. Preview timestamps keep their +original age: hover or keyboard focus reuses a fresh result and refreshes a stale +one. Stale status stays visible until then. Preview retention uses the same +24-hour expiry, build boundary, membership checks, and sign-out cleanup as other +saved reads. It does not add polling or fetch every PR on page load. + Embedded plan documents and preview work products are stripped from saved payloads. A denied task clears its UUID and identifier projections. Sign-out revocation markers contain no task content and remain after sign-out to reject stale records and delayed writes from older tabs. Session responses started before revocation cannot restore those reads, even before the storage event reaches the other tab. Cached agents retain only safe adapter display fields and an empty runtime configuration. Their configuration objects remain present for existing display consumers; provider credentials and environment values are excluded. diff --git a/tests/storybook-visual/link-previews.spec.ts b/tests/storybook-visual/link-previews.spec.ts index b5d32145df2..7d21fb31252 100644 --- a/tests/storybook-visual/link-previews.spec.ts +++ b/tests/storybook-visual/link-previews.spec.ts @@ -10,6 +10,25 @@ async function finished(page: import("@playwright/test").Page) { }).__STORYBOOK_ADDONS_CHANNEL__?.data.playFunctionThrewException ?? [])).toEqual([]); } +async function useRealPreviewRequests(page: import("@playwright/test").Page) { + // Keep the rest of Storybook's fixture API, but count preview requests at + // the browser HTTP boundary. The saved cache itself is production code. + await page.addInitScript(() => { + const realFetch = window.fetch.bind(window); + let activeFetch = realFetch; + Object.defineProperty(window, "fetch", { + configurable: true, + get: () => activeFetch, + set: (fixtureFetch: typeof fetch) => { + activeFetch = (input, init) => { + const url = input instanceof Request ? input.url : String(input); + return url.includes("/link-preview?") ? realFetch(input, init) : fixtureFetch(input, init); + }; + }, + }); + }); +} + for (const theme of ["light", "dark"]) { for (const state of ["open", "merged", "conflicts", "draft", "closed", "loading", "unavailable", "failed-read", "narrow"]) { test(`PR preview ${state} in ${theme}`, async ({ page }, testInfo) => { @@ -79,20 +98,7 @@ test("one deferred HTTP lookup updates repeated links and is reused on later hov }); // Neutral has no opening play step. Replace fixture fetch only for this read // so it crosses the browser's real HTTP boundary and can be counted. - await page.addInitScript(() => { - const realFetch = window.fetch.bind(window); - let activeFetch = realFetch; - Object.defineProperty(window, "fetch", { - configurable: true, - get: () => activeFetch, - set: (fixtureFetch: typeof fetch) => { - activeFetch = (input, init) => { - const url = input instanceof Request ? input.url : String(input); - return url.includes("/link-preview?") ? realFetch(input, init) : fixtureFetch(input, init); - }; - }, - }); - }); + await useRealPreviewRequests(page); await page.goto("/iframe.html?id=tasks-link-previews--neutral&viewMode=story"); await finished(page); expect(count).toBe(0); @@ -107,3 +113,57 @@ test("one deferred HTTP lookup updates repeated links and is reused on later hov await expect(page.getByRole("tooltip")).toBeVisible(); expect(count).toBe(1); }); + +for (const available of [true, false]) { + test(`PR cache survives reload and refreshes only on stale interest (available=${available})`, async ({ page }, testInfo) => { + let count = 0; + await page.route("**/api/companies/company-storybook/link-preview?*", async route => { + count++; + const canonicalUrl = new URL(route.request().url()).searchParams.get("url"); + await route.fulfill({ json: { + preview: available ? { + target: { provider: "github", kind: "pull_request", canonicalUrl, label: "acme/web#241" }, + title: "Saved PR status", state: count === 1 ? "merged" : "open", author: "octocat", updatedAt: null, + } : null, + maxAgeMs: 60_000, + } }); + }); + await useRealPreviewRequests(page); + await page.clock.install(); + await page.goto("/iframe.html?id=tasks-link-previews--reload-cache&viewMode=story"); + await finished(page); + const links = page.locator("a[data-link-state]"); + const expectedState = available ? "merged" : "unknown"; + await expect(links.first()).toHaveAttribute("data-link-state", "unknown"); + expect(count).toBe(0); + await links.first().hover({ position: { x: 5, y: 8 } }); + await expect(page.getByRole("tooltip")).toBeVisible(); + await expect(page.getByRole("status")).toHaveCount(0); + await expect(links.nth(1)).toHaveAttribute("data-link-state", expectedState); + expect(count).toBe(1); + // pagehide must flush even if reload beats the normal debounce. + await page.reload(); + await finished(page); + await expect(links.first()).toHaveAttribute("data-link-state", expectedState); + await expect(links.nth(1)).toHaveAttribute("data-link-state", expectedState); + expect(count).toBe(1); + await links.nth(1).focus(); + await expect(page.getByRole("tooltip")).toBeVisible(); + await expect(page.getByRole("status")).toHaveCount(0); + expect(count).toBe(1); + await page.screenshot({ path: testInfo.outputPath(`pr-cache-after-reload-${available}.png`), fullPage: true }); + await links.nth(1).blur(); + await page.mouse.move(0, 0); + await expect(page.getByRole("tooltip")).toBeHidden(); + await page.clock.setSystemTime(new Date(Date.now() + 120_000)); + await page.reload(); + await finished(page); + // Expiry preserves the last known display. It never fetches on render. + await expect(links.first()).toHaveAttribute("data-link-state", expectedState); + expect(count).toBe(1); + await links.first().focus(); + await expect(page.getByRole("tooltip")).toBeVisible(); + await expect.poll(() => count).toBe(2); + await expect(links.nth(1)).toHaveAttribute("data-link-state", available ? "open" : "unknown"); + }); +} diff --git a/ui/src/components/LinkPreview.tsx b/ui/src/components/LinkPreview.tsx index 9680ad31de3..e2c12e40c5f 100644 --- a/ui/src/components/LinkPreview.tsx +++ b/ui/src/components/LinkPreview.tsx @@ -6,6 +6,7 @@ import type { LinkPreview as Preview, LinkPreviewResult, LinkPreviewState, LinkP import { useLinkPreviewContext, type LinkPreviewContextValue } from "@/context/LinkPreviewContext"; import { timeAgo } from "@/lib/timeAgo"; import { retryTransientOnly } from "@/lib/query-client"; +import { queryKeys } from "@/lib/queryKeys"; export const linkPreviewStateLabel: Record = { unknown: "Status unknown", open: "Open", merged: "Merged", conflicts: "Merge conflicts", draft: "Draft", closed: "Closed", @@ -50,7 +51,7 @@ function ConnectedLinkPreview({ context, target, children }: { get contextElement() { return triggerRef.current ?? undefined; }, }), []); const query = useQuery({ - queryKey: ["link-preview", context.companyId, context.userId, target.canonicalUrl], + queryKey: queryKeys.linkPreview(context.companyId, context.userId, target.canonicalUrl), // Optional enrichment: a failed read is a cached absence, not a global error. queryFn: async (): Promise => { try { return await context.load(target); } catch { return { preview: null, maxAgeMs: 60_000 }; } diff --git a/ui/src/lib/persisted-query-cache.test.ts b/ui/src/lib/persisted-query-cache.test.ts index 368114e106d..0ff2ab40e55 100644 --- a/ui/src/lib/persisted-query-cache.test.ts +++ b/ui/src/lib/persisted-query-cache.test.ts @@ -38,6 +38,75 @@ function seed(client: QueryClient, userId = "alice", companyId = "company-one") afterEach(() => { for (const dispose of handles.splice(0)) dispose(); vi.useRealTimers(); vi.clearAllMocks(); }); describe("persisted read cache", () => { + it("restores learned PR status and negative reads without invalidating their demand-driven freshness", () => { + const first = setup(); seed(first.client); + const key = queryKeys.linkPreview("company-one", "alice", "https://github.com/acme/web/pull/241"); + const result = { preview: { title: "Saved PR status", state: "merged" }, maxAgeMs: 60_000 }; + first.client.setQueryData(key, result); + const absentKey = queryKeys.linkPreview("company-one", "alice", "https://github.com/acme/web/pull/242"); + first.client.setQueryData(absentKey, { preview: null, maxAgeMs: 60_000 }); + const updatedAt = first.client.getQueryState(key)!.dataUpdatedAt; + first.handle.flush(); + const restored = setup(first.storage); + expect(restored.client.getQueryData(key)).toEqual(result); + expect(restored.client.getQueryData(absentKey)).toEqual({ preview: null, maxAgeMs: 60_000 }); + expect(restored.client.getQueryState(key)).toMatchObject({ dataUpdatedAt: updatedAt, isInvalidated: false }); + }); + + it("persists PR reads only for the active viewer and an accessible company", () => { + const first = setup(); seed(first.client); + const url = "https://github.com/acme/web/pull/241"; + const result = { preview: { title: "Private PR" }, maxAgeMs: 60_000 }; + const forbidden = [ + queryKeys.linkPreview("company-one", "bob", url), + queryKeys.linkPreview("other-company", "alice", url), + ["link-preview", "company-one", null, url], + ["link-preview", "company-one", "alice"], + ["link-preview", "company-one", "alice", url, "extra"], + ]; + for (const key of forbidden) first.client.setQueryData(key, result); + first.handle.flush(); + expect([...first.storage.values.values()].join(" ")).not.toContain("Private PR"); + const restored = setup(first.storage); + for (const key of forbidden) expect(restored.client.getQueryData(key)).toBeUndefined(); + }); + + it("bounds saved PR reads and retains their original age through later saves", () => { + let now = Date.now(); + const first = setup(undefined, () => now); seed(first.client); + for (let i = 0; i < 120; i += 1) first.client.setQueryData( + queryKeys.linkPreview("company-one", "alice", `https://github.com/acme/web/pull/${i + 1}`), + { preview: { title: `PR ${i + 1}`, state: "open" }, maxAgeMs: 60_000 }, { updatedAt: now - 120 + i }); + first.handle.flush(); + const restored = setup(first.storage, () => now); + expect(restored.client.getQueryCache().findAll({ queryKey: ["link-preview"] })).toHaveLength(100); + expect(restored.client.getQueryData(queryKeys.linkPreview("company-one", "alice", "https://github.com/acme/web/pull/1"))).toBeUndefined(); + expect(restored.client.getQueryData(queryKeys.linkPreview("company-one", "alice", "https://github.com/acme/web/pull/120"))).toBeDefined(); + first.client.removeQueries({ queryKey: ["link-preview"] }); + now += 10 * 60_000; + first.handle.flush(); + expect(setup(first.storage, () => now).client.getQueryCache().findAll({ queryKey: ["link-preview"] })).toHaveLength(100); + now += QUERY_CACHE_MAX_AGE; + first.client.setQueryData(queryKeys.auth.session, session(), { updatedAt: now }); + first.client.setQueryData(queryKeys.companies.list("alice"), { companies: [{ id: "company-one" }] }, { updatedAt: now }); + first.handle.flush(); + expect(setup(first.storage, () => now).client.getQueryCache().findAll({ queryKey: ["link-preview"] })).toHaveLength(0); + }); + + it.each(["sign-out", "account-change", "membership-loss"])("removes persisted private PR reads on %s", action => { + const first = setup(); seed(first.client); + const key = queryKeys.linkPreview("company-one", "alice", "https://github.com/acme/private/pull/241"); + first.client.setQueryData(key, { preview: { title: "Private PR status" }, maxAgeMs: 60_000 }); + first.handle.flush(); + expect([...first.storage.values.values()].join(" ")).toContain("Private PR status"); + if (action === "sign-out") clearPersistedQueryCache(first.client); + if (action === "account-change") first.client.setQueryData(queryKeys.auth.session, session("bob")); + if (action === "membership-loss") first.client.setQueryData(queryKeys.companies.list("alice"), { companies: [] }); + first.handle.flush(); + expect([...first.storage.values.values()].join(" ")).not.toContain("Private PR status"); + expect(setup(first.storage).client.getQueryData(key)).toBeUndefined(); + }); + it.each([null, "", " "])("skips hydration and persistence without a build stamp (%s)", (buildId) => { const first = setup(); seed(first.client); first.handle.flush(); const saved = new Map(first.storage.values); diff --git a/ui/src/lib/persisted-query-cache.ts b/ui/src/lib/persisted-query-cache.ts index fee8fdf20f9..717935963c8 100644 --- a/ui/src/lib/persisted-query-cache.ts +++ b/ui/src/lib/persisted-query-cache.ts @@ -10,6 +10,7 @@ export const QUERY_CACHE_ACTIVE = `${QUERY_CACHE_PREFIX}active`; export const QUERY_CACHE_CLEARED = "paperclip:query-cache-cleared"; export const QUERY_CACHE_MAX_AGE = 24 * 60 * 60 * 1000; export const QUERY_CACHE_MAX_ISSUES = 20; +const MAX_LINK_PREVIEWS = 100; const MAX_RECORD_BYTES = 2 * 1024 * 1024; const REVISION_PREFIX = "paperclip:query-cache-revision:"; const controllers = new WeakMap void }>(); @@ -102,6 +103,8 @@ export function persistedQueryScope(key: readonly unknown[], data: unknown, user return typeof companyId === "string" && companyIds.includes(companyId) ? companyId : undefined; } if (typeof second !== "string" || !companyIds.includes(second)) return undefined; + if (root === "link-preview" && typeof key[3] === "string" + && hashKey(key) === hashKey(queryKeys.linkPreview(second, userId, key[3]))) return second; if (isLegacyInboxQuery(key) && Array.isArray(data)) return second; if (["sidebar-badges", "agents", "built-in-agents"].includes(String(root)) && key.length === 2) return second; if (root === "projects" && key.length <= 3) return second; @@ -304,7 +307,9 @@ export function startPersistedQueryCache(client: QueryClient, options: { .map(query => ({ queryKey: query.queryKey, queryHash: hashKey(query.queryKey), state: { data: queryDisplayData(query.queryKey, query.state.data, deniedIssues), dataUpdatedAt: query.state.dataUpdatedAt, dataUpdateCount: 1, error: null, errorUpdateCount: 0, errorUpdatedAt: 0, fetchFailureCount: 0, fetchFailureReason: null, - fetchMeta: null, isInvalidated: true, status: "success" as const, fetchStatus: "idle" as const, + // Optional link enrichment stays demand-driven. Keep its original read + // time so a fresh hover reuses it and a stale hover refreshes it. + fetchMeta: null, isInvalidated: query.queryKey[0] !== "link-preview", status: "success" as const, fetchStatus: "idle" as const, } })); hydrate(client, { mutations: [], queries }); }; @@ -361,7 +366,10 @@ export function startPersistedQueryCache(client: QueryClient, options: { .sort((a, b) => (visits.get(hashKey(b.queryKey)) ?? b.state.dataUpdatedAt) - (visits.get(hashKey(a.queryKey)) ?? a.state.dataUpdatedAt)); const issueIds = new Set(); for (const query of recent) if (issueIds.size < QUERY_CACHE_MAX_ISSUES) issueIds.add(issueIdentity(query)); + const previews = new Set([...merged.values()].filter(query => query.queryKey[0] === "link-preview") + .sort((a, b) => b.state.dataUpdatedAt - a.state.dataUpdatedAt).slice(0, MAX_LINK_PREVIEWS).map(query => hashKey(query.queryKey))); const queries = [...merged.values()].filter(query => allowedIssue(query) + && (query.queryKey[0] !== "link-preview" || previews.has(hashKey(query.queryKey))) && !(query.queryKey[0] === "issues" && query.queryKey[1] === "detail" && !issueIds.has(issueIdentity(query)))) .map(query => projectQuery(query, userId)).filter(query => query.state.data !== undefined); try { diff --git a/ui/src/lib/queryKeys.ts b/ui/src/lib/queryKeys.ts index 8e0736915a8..3b78266646c 100644 --- a/ui/src/lib/queryKeys.ts +++ b/ui/src/lib/queryKeys.ts @@ -1,4 +1,6 @@ export const queryKeys = { + linkPreview: (companyId: string, userId: string | null, url: string) => + ["link-preview", companyId, userId, url] as const, workspacePreferences: { mine: (userId: string | null) => ["workspace-preferences", userId] as const, }, diff --git a/ui/storybook/stories/tasks/link-previews.stories.tsx b/ui/storybook/stories/tasks/link-previews.stories.tsx index 45a2b152dc5..b6df1491d8a 100644 --- a/ui/storybook/stories/tasks/link-previews.stories.tsx +++ b/ui/storybook/stories/tasks/link-previews.stories.tsx @@ -1,15 +1,18 @@ -import { useState } from "react"; +import { useEffect, useState } from "react"; import type { Meta, StoryObj } from "@storybook/react-vite"; import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; import { recognizeLinkPreview, type LinkPreviewState } from "@paperclipai/shared"; import { MarkdownBody } from "@/components/MarkdownBody"; import { LinkPreviewProvider } from "@/context/LinkPreviewContext"; +import { startPersistedQueryCache } from "@/lib/persisted-query-cache"; +import { queryKeys } from "@/lib/queryKeys"; import { prepareFixtureApi } from "../../support/capability-gaps/api"; interface PreviewArgs { state: LinkPreviewState; mode: "ready" | "loading" | "unavailable" | "failure"; longTitle: boolean } -function PreviewExample(_args: PreviewArgs) { - const [client] = useState(() => new QueryClient()); +function PreviewExample({ client: restoredClient }: PreviewArgs & { client?: QueryClient }) { + const [localClient] = useState(() => new QueryClient()); + const client = restoredClient ?? localClient; return
@@ -29,6 +32,19 @@ function PreviewExample(_args: PreviewArgs) { ; } +function PersistedPreviewExample(args: PreviewArgs) { + const [client] = useState(() => new QueryClient()); + const [ready, setReady] = useState(false); + useEffect(() => { + client.setQueryData(queryKeys.auth.session, { user: { id: "user-board" } }); + client.setQueryData(queryKeys.companies.list("user-board"), { companies: [{ id: "company-storybook" }] }); + const cache = startPersistedQueryCache(client, { buildId: "link-preview-story-v1" }); + setReady(true); + return () => { cache.dispose(); client.clear(); }; + }, [client]); + return ready ? : null; +} + const meta = { id: "tasks-link-previews", title: "Tasks and conversations/Task creation and lifecycle/Link previews", @@ -60,6 +76,10 @@ export default meta; type Story = StoryObj; export const Neutral: Story = {}; +export const ReloadCache: Story = { + render: args => , + parameters: { paperclip: { notes: "Production browser read-cache persistence with fixture account/company identity. Load a PR, then reload this iframe to retain the learned status. Fresh previews reuse their original read time; stale previews refresh only on hover or focus. No live provider authorization is claimed." } }, +}; export const Open: Story = {}; export const Merged: Story = { args: { state: "merged" } }; export const Conflicts: Story = { args: { state: "conflicts" } };