diff --git a/doc/DEVELOPING.md b/doc/DEVELOPING.md index 2e8540363a0..59489b8f097 100644 --- a/doc/DEVELOPING.md +++ b/doc/DEVELOPING.md @@ -4555,3 +4555,15 @@ secret handling, initialization order and migration interfaces. Read surfaces use `useQueryView` and `QueryErrorState`. Transient refetch failures keep loaded data visible. An initial outage keeps a quiet placeholder until recovery. Authorization, missing resources and validation failures still show readable errors. Exhausted first reads on a reachable server offer Retry, including task filters and hierarchy reads. Health probes and the socket own app-wide recovery; a successful component load may be an older in-flight response or a local no-op. Sensitive manual reads use `useResilientFetch`; browser viewer URLs and pairing previews remain in component memory and never enter a shared cache. Run `node scripts/check-query-error-rendering.mjs`. The guard now enforces across `ui/src`, including multiline expressions, and CI runs it as a policy check. Its small allowlist covers standalone preview/performance fixtures, transcript protocol result flags, and the existing account-resolution and Copilot discovery retry owners. These are not ordinary read-error renders. Additions require a specific reason; do not allowlist a page to hide an outage regression. + +The browser saves a small read cache in `paperclip:query-cache:v1:*` local storage records. It contains session display identity, health and shell preferences, company and sidebar data, agent/project lists, plugin UI contribution metadata, the first inbox page (including up to 100 rows per legacy Inbox query), and up to 20 recently opened tasks. Records belong to one user and company, expire 24 hours after each read, and are discarded when the build changes. Normal in-memory query garbage collection does not erase saved recent tasks or extend their expiry. Builds without a source commit stamp skip read-cache hydration and persistence, but still revoke and clear saved reads on sign-out. Plugin contributions use the existing instance-wide query inside the user's root record. Session tokens, bootstrap payloads, documents, comments, browser viewer capabilities, pairing data, other queries and mutations are excluded. Task content in this allowlist remains on the browser until expiry or sign-out; use the normal sign-out action to clear it across tabs. Account changes also detach pending shared GETs so a new user cannot join the previous user’s private reads. Editors that need fields excluded from the display cache, including bulk agent settings, fetch complete current data before opening. + +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`. + +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. + +A restored task shows its header and properties while uncached history is still loading. History is not persisted, and its normal placeholder remains until reads recover. The header stays visible across recovery. + +Query-cache cleanup snapshots storage keys before deletion because native storage can reorder them as records are removed. Flush cleanup affects only its owning account and yields if another account becomes active. diff --git a/tests/e2e/task-loading-frame.spec.ts b/tests/e2e/task-loading-frame.spec.ts index e784e61dfc1..f452c04a3c6 100644 --- a/tests/e2e/task-loading-frame.spec.ts +++ b/tests/e2e/task-loading-frame.spec.ts @@ -234,6 +234,9 @@ test("a seeded Classic loading title uses the resolved title typography", async await page.getByRole("link").filter({ hasText: fixture.title }).first().click(); await expect(page.getByTestId("issue-detail-header").getByRole("heading", { name: fixture.title, exact: true })).toBeVisible(); await page.route("**/api/issues/*", async route => { await coreReady; await route.continue(); }); + // Keep the recent-task title seed, but exercise the loading frame without + // a persisted full task that would correctly reveal the resolved header. + await page.addInitScript(() => Object.keys(localStorage).filter(key => key.startsWith("paperclip:query-cache:v1:")).forEach(key => localStorage.removeItem(key))); await page.reload({ waitUntil: "domcontentloaded" }); const loadingTitle = page.getByRole("heading", { name: fixture.title, exact: true }); await expect(loadingTitle).toBeVisible(); diff --git a/ui/src/api/agents.ts b/ui/src/api/agents.ts index c6e4e7dce54..07358ef8377 100644 --- a/ui/src/api/agents.ts +++ b/ui/src/api/agents.ts @@ -39,7 +39,7 @@ import type { AgentApiKeyScope, } from "@paperclipai/shared"; import { isUuidLike, normalizeAgentUrlKey } from "@paperclipai/shared"; -import { ApiError, api } from "./client"; +import { ApiError, api, type RequestOptions } from "./client"; export interface AgentKey { id: string; @@ -127,9 +127,9 @@ export const agentsApi = { org: (companyId: string) => api.get(`/companies/${companyId}/org`), listConfigurations: (companyId: string) => api.get[]>(`/companies/${companyId}/agent-configurations`), - get: async (id: string, companyId?: string) => { + get: async (id: string, companyId?: string, options?: RequestOptions) => { try { - return await api.get(agentPath(id, companyId)); + return await api.get(agentPath(id, companyId), options); } catch (error) { // Backward-compat fallback: if backend shortname lookup reports ambiguity, // resolve using company agent list while ignoring terminated agents. @@ -145,12 +145,12 @@ export const agentsApi = { const urlKey = normalizeAgentUrlKey(id); if (!urlKey) throw error; - const agents = await api.get(`/companies/${companyId}/agents`); + const agents = await api.get(`/companies/${companyId}/agents`, options); const matches = agents.filter( (agent) => agent.status !== "terminated" && normalizeAgentUrlKey(agent.urlKey) === urlKey, ); if (matches.length !== 1) throw error; - return api.get(agentPath(matches[0]!.id, companyId)); + return api.get(agentPath(matches[0]!.id, companyId), options); } }, getConfiguration: (id: string, companyId?: string) => diff --git a/ui/src/api/client.ts b/ui/src/api/client.ts index b0128490f47..1eee1bc3b2f 100644 --- a/ui/src/api/client.ts +++ b/ui/src/api/client.ts @@ -262,6 +262,11 @@ export function detachInflightGet(path: string): void { inflightGets.delete(path); } +/** End the sharing scope when identity changes; existing callers still settle. */ +export function detachInflightGets(): void { + inflightGets.clear(); +} + function isRequestOptions(value: unknown): value is RequestOptions { return typeof value === "object" && value !== null && "signal" in value; } diff --git a/ui/src/api/companies-query.ts b/ui/src/api/companies-query.ts index 5eff1cb0c21..c072208bec2 100644 --- a/ui/src/api/companies-query.ts +++ b/ui/src/api/companies-query.ts @@ -5,6 +5,7 @@ import { companiesApi } from "./companies"; import { ApiError } from "./client"; import { queryKeys } from "../lib/queryKeys"; import { useBootstrapQuery } from "./bootstrap-query"; +import { hasUnverifiedSession } from "../lib/cached-session"; export type CompanyListResult = { companies: Company[]; unauthorized: boolean }; @@ -140,7 +141,7 @@ export async function resolveAccountUserId(queryClient: QueryClient): Promise ({ userId: "user-1", pending: null as Promise<{ user: { id: string } } | null> | null })); @@ -217,6 +220,56 @@ describe("InlineEditor", () => { act(() => root.unmount()); }); + it.each([false, true])("checks a restored draft against a fresh field after session verification (multiline=%s)", async (multiline) => { + const key = "paperclip:inline-draft:v2:user-1:cached-field"; + localStorage.setItem(key, JSON.stringify({ version: 1, draft: "My draft B", base: "Cached A" })); + const client = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + client.setQueryData(queryKeys.auth.session, { user: { id: "user-1" } }); + const cache = startPersistedQueryCache(client, { storage: null, buildId: "test", eventTarget: null }); + markCachedSession(client); + let releaseSession!: () => void; + mockSession.pending = new Promise(resolve => { releaseSession = () => resolve({ user: { id: "user-1" } }); }); + let releaseRead!: (value: string) => void; + const readCurrentValue = vi.fn<() => Promise>().mockImplementationOnce(() => new Promise(resolve => { releaseRead = resolve; })).mockResolvedValue("New server C"); + const onSave = vi.fn().mockResolvedValue(undefined); + const root = createRoot(container); + const render = (value: string) => root.render( + + ); + try { + await act(async () => render("Cached A")); + expect(onSave).not.toHaveBeenCalled(); + expect(readCurrentValue).not.toHaveBeenCalled(); + await act(async () => releaseSession()); + expect(readCurrentValue).toHaveBeenCalledOnce(); + expect(onSave).not.toHaveBeenCalled(); + await act(async () => releaseRead("New server C")); + await act(async () => render("New server C")); + expect(container.querySelector("textarea")?.value).toBe("My draft B"); + expect(localStorage.getItem(key)).toContain("My draft B"); + expect(onSave).not.toHaveBeenCalled(); + if (!multiline) { + expect(container.querySelector("textarea")?.getAttribute("aria-invalid")).toBe("true"); + expect(container.querySelector("textarea")?.getAttribute("aria-description")).toContain("This changed since you loaded it"); + await act(async () => container.querySelector("textarea")!.dispatchEvent(new KeyboardEvent("keydown", { key: "Enter", bubbles: true }))); + expect(readCurrentValue).toHaveBeenCalledTimes(2); + expect(onSave).not.toHaveBeenCalled(); + } + } finally { act(() => root.unmount()); cache.dispose(); client.clear(); mockSession.pending = null; } + }); + + it("replays a restored draft after a fresh field confirms its base", async () => { + localStorage.setItem("paperclip:inline-draft:v2:user-1:confirmed-field", JSON.stringify({ version: 1, draft: "Confirmed draft", base: "Original" })); + const readCurrentValue = vi.fn().mockResolvedValue("Original"); + const onSave = vi.fn().mockResolvedValue(undefined); + const root = createRoot(container); + try { + await act(async () => root.render(signedIn("user-1", ))); + expect(readCurrentValue).toHaveBeenCalledOnce(); + expect(onSave).toHaveBeenCalledWith("Confirmed draft"); + } finally { act(() => root.unmount()); } + }); + it("restores an unsaved edit and saves it when the server value has not moved", async () => { localStorage.setItem("paperclip:inline-draft:v2:user-1:issue-1:description", JSON.stringify({ version: 1, draft: "Typed before reload", base: "Original" })); const onSave = vi.fn().mockResolvedValue(undefined); @@ -255,6 +308,36 @@ describe("InlineEditor", () => { } }); + it("releases a cancelled description debounce when switching cached tasks and keeps its stored draft", async () => { + vi.useFakeTimers(); + const client = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + client.setQueryData(queryKeys.auth.session, { user: { id: "user-1" } }); + const saveA = vi.fn().mockResolvedValue(undefined); + const saveB = vi.fn().mockResolvedValue(undefined); + const root = createRoot(container); + const renderTask = (id: string, onSave: typeof saveA) => root.render( + + + , + ); + try { + await act(async () => renderTask("a", saveA)); + act(() => enterMultilineEdit(container)); + act(() => { + const textarea = container.querySelector('[data-testid="multiline-md-mock"]')!; + textarea.focus(); + setNativeTextareaValue(textarea, "A draft before navigation"); + }); + expect(hasPendingWrites()).toBe(true); + await act(async () => renderTask("b", saveB)); + expect(hasPendingWrites()).toBe(false); + await act(async () => { await vi.advanceTimersByTimeAsync(1_000); }); + expect(saveA).not.toHaveBeenCalled(); + expect(saveB).not.toHaveBeenCalled(); + expect(localStorage.getItem("paperclip:inline-draft:v2:user-1:task-a:description")).toContain("A draft before navigation"); + } finally { act(() => root.unmount()); client.clear(); localStorage.clear(); vi.useRealTimers(); } + }); + it("restores an unsaved edit without overwriting a newer server value", async () => { localStorage.setItem("paperclip:inline-draft:v2:user-1:issue-2:description", JSON.stringify({ version: 1, draft: "My old edit", base: "Original" })); const onSave = vi.fn().mockResolvedValue(undefined); @@ -419,6 +502,30 @@ describe("InlineEditor", () => { }); }); + it("releases update blocking when a newer multiline edit is undone after an older save completes", async () => { + vi.useFakeTimers(); + const root = createRoot(container); + let complete!: () => void; + const onSave = vi.fn().mockImplementationOnce(() => new Promise(resolve => { complete = resolve; })); + try { + await act(async () => root.render()); + const textarea = container.querySelector("textarea")!; + act(() => { textarea.focus(); setNativeTextareaValue(textarea, "First edit"); }); + await act(async () => { await vi.advanceTimersByTimeAsync(1000); }); + expect(onSave).toHaveBeenCalledWith("First edit"); + act(() => setNativeTextareaValue(textarea, "Newer edit")); + await act(async () => { complete(); await Promise.resolve(); }); + expect(container.textContent).toContain("Saved"); + expect(hasPendingWrites()).toBe(true); + act(() => setNativeTextareaValue(textarea, "Original")); + expect(container.textContent).toContain("Saved"); + expect(hasPendingWrites()).toBe(false); + await act(async () => { await vi.advanceTimersByTimeAsync(2000); }); + expect(onSave).toHaveBeenCalledTimes(1); + expect(container.textContent).not.toContain("Saved"); + } finally { act(() => root.unmount()); vi.useRealTimers(); } + }); + it("multiline nullable clear uses autosave path (shows Saved after blur)", async () => { const onSave = vi.fn().mockResolvedValue(undefined); const root = createRoot(container); diff --git a/ui/src/components/InlineEditor.tsx b/ui/src/components/InlineEditor.tsx index dce54904685..69ae83adf70 100644 --- a/ui/src/components/InlineEditor.tsx +++ b/ui/src/components/InlineEditor.tsx @@ -2,6 +2,7 @@ import { useState, useRef, useEffect, useCallback, useId } from "react"; import { cn } from "../lib/utils"; import { MarkdownBody, type MarkdownExternalReferenceMap } from "./MarkdownBody"; import { MarkdownEditor, type MarkdownEditorRef, type MentionOption } from "./MarkdownEditor"; +import { ApiError } from "../api/client"; import { useDurableAutosave } from "../hooks/useDurableAutosave"; import { useDraftOwner } from "../hooks/useDraftOwner"; import { loadStructuredDraft, saveStructuredDraft } from "../lib/composer-draft"; @@ -40,6 +41,8 @@ interface InlineEditorProps { * outage does not lose them. A stored edit is restored and saved on mount. */ draftKey?: string; + /** Fresh field read used before replaying a restored draft; cached values are display only. */ + readCurrentValue?: () => Promise; } /** Shared padding so display and edit modes occupy the exact same box. */ @@ -88,6 +91,7 @@ function OwnedInlineEditor({ defaultEditing = false, onEditingChange, draftKey, + readCurrentValue, draftOwner, }: InlineEditorProps & { draftOwner: string | undefined }) { const [editing, setEditing] = useState(false); @@ -106,6 +110,10 @@ function OwnedInlineEditor({ const [draft, setDraft] = useState(value); const unownedDraft = useRef(null); const lastPropValueRef = useRef(value); + const lastDraftKeyRef = useRef(draftKey); + const restoredBaseRef = useRef(null); + const readCurrentValueRef = useRef(readCurrentValue); + readCurrentValueRef.current = readCurrentValue; const inputRef = useRef(null); const markdownRef = useRef(null); const autosaveDebounceRef = useRef | null>(null); @@ -120,7 +128,19 @@ function OwnedInlineEditor({ await onSave(next); }, }); - const { markDirty, reset, run: runAutosave } = autosave; + const { markDirty, clearDirty, reset, run: runAutosave } = autosave; + const runInlineAutosave = useCallback((next: string) => { + const restored = restoredBaseRef.current; + const readCurrent = readCurrentValueRef.current; + return runAutosave(next, { + beforeSave: restored && restored.draft.trim() === next && readCurrent ? async () => { + const current = await readCurrent(); + if (current !== restored.base && current.trim() !== next) { + throw new ApiError("This changed since you loaded it. Your draft is kept for review.", 409, {}); + } + } : undefined, + }); + }, [runAutosave]); const autosaveState = autosave.status; const autosaveStateRef = useRef(autosaveState); autosaveStateRef.current = autosaveState; @@ -147,6 +167,7 @@ function OwnedInlineEditor({ }, [clearStoredDraft, storageKey, value]); const changeDraft = useCallback((next: string) => { + restoredBaseRef.current = null; setDraft(next); if (draftOwner === undefined) unownedDraft.current = { version: 1, draft: next, base: value }; if (!storageKey) return; @@ -156,14 +177,15 @@ function OwnedInlineEditor({ useEffect(() => { const previousValue = lastPropValueRef.current; + const sameTarget = lastDraftKeyRef.current === draftKey; lastPropValueRef.current = value; + lastDraftKeyRef.current = draftKey; + if (sameTarget && previousValue === value) return; setDraft((currentDraft) => { - if (multiline && multilineFocused && currentDraft !== previousValue) { - return currentDraft; - } + if (sameTarget && (editing || multilineEditing) && currentDraft !== previousValue) return currentDraft; return value; }); - }, [value, multiline, multilineFocused]); + }, [value, draftKey, editing, multilineEditing]); // An edit that never saved (reload, crash, outage) comes back. It saves on // its own only when the server value is still the one it was based on; @@ -172,6 +194,7 @@ function OwnedInlineEditor({ useEffect(() => { if (!storageKey || restoredDraftRef.current === storageKey) return; restoredDraftRef.current = storageKey; + restoredBaseRef.current = null; if (unownedDraft.current) { if (draft.trim() !== value.trim()) saveStructuredDraft(storageKey, { ...unownedDraft.current, draft }); else clearStoredDraft(); @@ -184,6 +207,7 @@ function OwnedInlineEditor({ clearStoredDraft(); return; } + restoredBaseRef.current = stored; setDraft(stored.draft); if (multiline) { setMultilineEditing(true); @@ -191,7 +215,7 @@ function OwnedInlineEditor({ } else { setEditing(true); } - if (stored.base === value) void runAutosave(stored.draft.trim()); + if (stored.base === value) void runInlineAutosave(stored.draft.trim()); // eslint-disable-next-line react-hooks/exhaustive-deps }, [storageKey]); @@ -284,10 +308,10 @@ function OwnedInlineEditor({ if (!multiline) setEditing(false); return; } - const result = await runAutosave(valueToSave); + const result = await runInlineAutosave(valueToSave); // A single-line edit stays open until it is saved, so its text is not lost. if (!multiline && result.kind === "saved") setEditing(false); - }, [clearStoredDraft, draft, draftKey, multiline, nullable, runAutosave, value]); + }, [clearStoredDraft, draft, draftKey, multiline, nullable, runInlineAutosave, value]); /** Multiline blur/submit: show autosave indicator when persisting */ const finalizeMultilineBlurOrSubmit = useCallback(() => { @@ -348,9 +372,8 @@ function OwnedInlineEditor({ const trimmed = draft.trim(); // Nullable: empty draft can still be a real edit (clearing); only skip debounce when unchanged or empty is invalid. if (trimmed === value || (!trimmed && !nullable)) { - if (autosaveStateRef.current !== "saved") { - reset(); - } + if (autosaveStateRef.current === "saved") clearDirty(); + else reset(); return; } markDirty(); @@ -366,7 +389,7 @@ function OwnedInlineEditor({ clearTimeout(autosaveDebounceRef.current); } }; - }, [commit, draft, markDirty, multiline, multilineFocused, nullable, reset, value]); + }, [clearDirty, commit, draft, markDirty, multiline, multilineFocused, nullable, reset, value]); if (multiline) { const previewValue = autosaveState === "saved" || autosaveState === "idle" ? draft : value; @@ -486,6 +509,8 @@ function OwnedInlineEditor({