From 942c8c4c40bfc2bfdca3ebcef9553d061ebf0dba Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Sun, 30 Aug 2026 18:13:31 -0700 Subject: [PATCH 1/4] fix(share): invalidate stale short links --- .../useSharing.shortUrlLifecycle.test.tsx | 241 ++++++++++++++++++ packages/ui/hooks/useSharing.ts | 55 +++- 2 files changed, 288 insertions(+), 8 deletions(-) create mode 100644 packages/ui/hooks/useSharing.shortUrlLifecycle.test.tsx diff --git a/packages/ui/hooks/useSharing.shortUrlLifecycle.test.tsx b/packages/ui/hooks/useSharing.shortUrlLifecycle.test.tsx new file mode 100644 index 000000000..8c84710b8 --- /dev/null +++ b/packages/ui/hooks/useSharing.shortUrlLifecycle.test.tsx @@ -0,0 +1,241 @@ +import React, { useState } from 'react'; +import { afterEach, describe, expect, test } from 'bun:test'; +import { act } from 'react'; +import { createRoot, type Root } from 'react-dom/client'; +import { compress, decompress } from '@plannotator/core/compress'; +import { encrypt } from '@plannotator/core/crypto'; +import { useSharing } from './useSharing'; +import { AnnotationType, type Annotation, type ImageAttachment } from '../types'; +import type { SharePayload } from '../utils/sharing'; + +const hasDom = typeof document !== 'undefined'; +const originalFetch = globalThis.fetch; +let root: Root | null = null; +let host: HTMLDivElement | null = null; + +afterEach(() => { + if (!hasDom) return; + act(() => root?.unmount()); + root = null; + host?.remove(); + host = null; + globalThis.fetch = originalFetch; + window.history.replaceState({}, '', '/'); +}); + +type SharingResult = ReturnType; + +interface SharingControls { + readonly setMarkdown: React.Dispatch>; + readonly setAnnotations: React.Dispatch>; + readonly setAttachments: React.Dispatch>; +} + +interface HarnessCapture { + result: SharingResult | null; + controls: SharingControls | null; +} + +function Harness({ + contentRevision, + onResult, + onControls, +}: { + contentRevision: number; + onResult: (result: SharingResult) => void; + onControls: (controls: SharingControls) => void; +}) { + const [markdown, setMarkdown] = useState(''); + const [annotations, setAnnotations] = useState([]); + const [attachments, setAttachments] = useState([]); + const [rawHtml, setRawHtml] = useState(''); + const [shareHtml, setShareHtml] = useState(''); + const [, setRenderAs] = useState<'markdown' | 'html'>('markdown'); + const result = useSharing( + markdown, + annotations, + attachments, + setMarkdown, + setAnnotations, + setAttachments, + undefined, + 'https://share.example.test', + 'https://paste.example.test', + rawHtml || undefined, + async () => shareHtml || rawHtml, + setRawHtml, + setShareHtml, + setRenderAs, + contentRevision, + ); + + onResult(result); + onControls({ setMarkdown, setAnnotations, setAttachments }); + return null; +} + +async function waitFor(condition: () => boolean): Promise { + for (let attempt = 0; attempt < 50 && !condition(); attempt += 1) { + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 0)); + }); + } + expect(condition()).toBe(true); +} + +async function installIncomingPaste( + payload: SharePayload, + pasteId: string, +): Promise<{ readonly incomingUrl: string; readonly getPasteResponse: Response }> { + const compressed = await compress(payload); + const encrypted = await encrypt(compressed); + window.location.href = `http://localhost/p/${pasteId}#key=${encrypted.key}`; + return { + incomingUrl: window.location.href, + getPasteResponse: Response.json({ data: encrypted.ciphertext }), + }; +} + +async function mountHarness( + contentRevision: number, + capture: HarnessCapture, +): Promise { + host = document.createElement('div'); + document.body.appendChild(host); + root = createRoot(host); + await act(async () => { + renderHarness(contentRevision, capture); + await new Promise((resolve) => setTimeout(resolve, 0)); + }); +} + +function renderHarness(contentRevision: number, capture: HarnessCapture): void { + root?.render( + + { capture.result = result; }} + onControls={(controls) => { capture.controls = controls; }} + /> + , + ); +} + +describe.if(hasDom)('useSharing short URL lifecycle', () => { + test('preserves an incoming short URL through hydration, then invalidates immutable snapshots after material edits', async () => { + const payload: SharePayload = { + p: '# Shared plan\n\nOriginal document', + a: [['C', 'Original document', 'Initial feedback', null]], + g: [['data:image/png;base64,aW5pdGlhbA==', 'initial.png']], + }; + const incoming = await installIncomingPaste(payload, 'AbCd1234'); + let postCount = 0; + // SAFETY: This boundary fake implements the fetch calls exercised by useSharing and returns real Response values. + globalThis.fetch = (async (input, init) => { + const url = String(input); + if (url.endsWith('/api/paste/AbCd1234')) return incoming.getPasteResponse.clone(); + if (url.endsWith('/api/paste') && init?.method === 'POST') { + postCount += 1; + return Response.json({ id: `Local00${postCount}` }, { status: 201 }); + } + return Response.json({ error: 'Unexpected request' }, { status: 500 }); + }) as typeof fetch; + + const capture: HarnessCapture = { + result: null, + controls: null, + }; + await mountHarness(0, capture); + await waitFor(() => capture.result?.isLoadingShared === false); + + expect(capture.result?.shortShareUrl).toBe(incoming.incomingUrl); + expect(capture.result?.isSharedSession).toBe(true); + await waitFor(() => Boolean(capture.result?.shareUrl)); + const hydratedShareUrl = capture.result?.shareUrl ?? ''; + + await act(async () => { + capture.controls?.setAnnotations((current) => [...current, { + id: 'teammate-annotation', + blockId: 'block-1', + startOffset: 0, + endOffset: 8, + type: AnnotationType.COMMENT, + originalText: 'Original', + text: 'Teammate feedback', + createdA: 2, + }]); + }); + expect(capture.result?.shortShareUrl).toBe(''); + expect(postCount).toBe(0); + + await waitFor(() => Boolean(capture.result?.shareUrl) && capture.result?.shareUrl !== hydratedShareUrl); + const annotatedPayload = await decompress(capture.result?.shareUrl.split('#')[1] ?? ''); + // SAFETY: generateShareUrl produced this compressed SharePayload in the same hook. + const annotatedSharePayload = annotatedPayload as SharePayload; + expect(annotatedSharePayload.a).toHaveLength(2); + + let firstLocalUrl: string | null = null; + await act(async () => { + firstLocalUrl = await capture.result?.generateShortUrl() ?? null; + }); + expect(firstLocalUrl).toContain('/p/Local001#key='); + expect(capture.result?.shortShareUrl).toBe(firstLocalUrl); + + await act(async () => { + renderHarness(0, capture); + await new Promise((resolve) => setTimeout(resolve, 0)); + }); + expect(capture.result?.shortShareUrl).toBe(firstLocalUrl); + + await act(async () => { + capture.controls?.setAttachments((current) => [...current, { + path: 'data:image/png;base64,bmV3', + name: 'new.png', + }]); + }); + expect(capture.result?.shortShareUrl).toBe(''); + expect(postCount).toBe(1); + + await act(async () => { + await capture.result?.generateShortUrl(); + }); + expect(capture.result?.shortShareUrl).toContain('/p/Local002#key='); + + await act(async () => { + capture.controls?.setMarkdown('# Shared plan\n\nEdited document'); + }); + expect(capture.result?.shortShareUrl).toBe(''); + expect(postCount).toBe(2); + }); + + test('invalidates a hydrated HTML short URL when the portable content revision changes', async () => { + const payload: SharePayload = { + p: '', + a: [], + h: '

Shared HTML

', + r: 'html', + }; + const incoming = await installIncomingPaste(payload, 'Html1234'); + // SAFETY: This boundary fake implements the single paste fetch exercised by the hydrated HTML share. + globalThis.fetch = (async (input) => { + if (String(input).endsWith('/api/paste/Html1234')) return incoming.getPasteResponse.clone(); + return Response.json({ error: 'Unexpected request' }, { status: 500 }); + }) as typeof fetch; + + const capture: HarnessCapture = { + result: null, + controls: null, + }; + await mountHarness(0, capture); + await waitFor(() => capture.result?.isLoadingShared === false); + expect(capture.result?.shortShareUrl).toBe(incoming.incomingUrl); + expect(capture.result?.shareUrl).toBe(''); + + await act(async () => { + renderHarness(1, capture); + await new Promise((resolve) => setTimeout(resolve, 0)); + }); + expect(capture.result?.shortShareUrl).toBe(''); + expect(capture.result?.shareUrl).toBe(''); + }); +}); diff --git a/packages/ui/hooks/useSharing.ts b/packages/ui/hooks/useSharing.ts index fd57df680..a2d68a776 100644 --- a/packages/ui/hooks/useSharing.ts +++ b/packages/ui/hooks/useSharing.ts @@ -76,6 +76,11 @@ interface UseSharingResult { clearShareLoadError: () => void; } +type ShortShareUrlLifecycle = + | { readonly _tag: 'none' } + | { readonly _tag: 'incoming-hydration' } + | { readonly _tag: 'generating'; readonly requestContext: object } + | { readonly _tag: 'associated'; readonly requestContext: object }; // Share payloads are base64url-encoded deflate output: charset [A-Za-z0-9_-], // realistically >=30 chars, and virtually always mixed-case because deflate @@ -133,6 +138,7 @@ export function useSharing( ]); const latestShareRequestContextRef = useRef(shareRequestContext); latestShareRequestContextRef.current = shareRequestContext; + const shortShareUrlLifecycleRef = useRef({ _tag: 'none' }); const clearPendingSharedAnnotations = useCallback(() => { setPendingSharedAnnotations(null); @@ -148,6 +154,10 @@ export function useSharing( const pathMatch = window.location.pathname.match(/^\/p\/([A-Za-z0-9]{6,16})$/); if (pathMatch) { const pasteId = pathMatch[1]; + // Capture before the async fetch. React Strict Mode may invoke this + // mount effect twice; the first completion removes /p/ from + // history, but both completions must preserve the original short URL. + const incomingShortUrl = window.location.href; // Extract key and optional paste origin from fragment: #key=&paste= const fragment = window.location.hash.slice(1); @@ -180,7 +190,8 @@ export function useSharing( setPendingSharedAnnotations(restoredAnnotations); setIsSharedSession(true); - setShortShareUrl(window.location.href); + shortShareUrlLifecycleRef.current = { _tag: 'incoming-hydration' }; + setShortShareUrl(incomingShortUrl); onSharedLoad?.(); // Remove the /p/ path from browser history so a refresh doesn't @@ -294,17 +305,35 @@ export function useSharing( refreshShareUrl(); }, [refreshShareUrl]); - // Clear stale short URL when content changes (does NOT auto-regenerate — - // the user must explicitly click "Create short link" again). - // Skip on shared session load — the incoming short URL must survive. - const isSharedRef = useRef(false); + // An incoming short URL becomes associated with the fully hydrated share + // context on its first committed render. From then on it follows the same + // lifecycle as a locally generated URL: any shareable-content change makes + // the immutable paste stale, so discard the URL without auto-uploading a + // replacement. Markdown users can still use the fresh hash URL; creating a + // new short link remains an explicit action. useEffect(() => { - if (isSharedSession) { isSharedRef.current = true; return; } - if (isSharedRef.current) { isSharedRef.current = false; return; } + const lifecycle = shortShareUrlLifecycleRef.current; + if (lifecycle._tag === 'incoming-hydration') { + if (!shortShareUrl) return; + shortShareUrlLifecycleRef.current = { + _tag: 'associated', + requestContext: shareRequestContext, + }; + return; + } + if ( + (lifecycle._tag === 'generating' || lifecycle._tag === 'associated') + && lifecycle.requestContext === shareRequestContext + ) { + return; + } + if (lifecycle._tag === 'none' && !shortShareUrl) return; + + shortShareUrlLifecycleRef.current = { _tag: 'none' }; setIsGeneratingShortUrl(false); setShortShareUrl(''); setShortUrlError(''); - }, [markdown, annotations, globalAttachments, rawHtml, isSharedSession, contentRevision]); + }, [shareRequestContext, shortShareUrl]); /** * Generate a short URL via the paste service. @@ -318,6 +347,10 @@ export function useSharing( setIsGeneratingShortUrl(true); setShortUrlError(''); const requestContext = shareRequestContext; + shortShareUrlLifecycleRef.current = { + _tag: 'generating', + requestContext, + }; try { const htmlForShare = rawHtml @@ -334,15 +367,21 @@ export function useSharing( if (latestShareRequestContextRef.current !== requestContext) return null; if (result) { + shortShareUrlLifecycleRef.current = { + _tag: 'associated', + requestContext, + }; setShortShareUrl(result.shortUrl); return result.shortUrl; } else { + shortShareUrlLifecycleRef.current = { _tag: 'none' }; setShortShareUrl(''); setShortUrlError('Short URL service unavailable'); return null; } } catch (e) { if (latestShareRequestContextRef.current !== requestContext) return null; + shortShareUrlLifecycleRef.current = { _tag: 'none' }; setShortShareUrl(''); setShortUrlError(e instanceof Error ? e.message : 'Failed to generate short URL'); return null; From bcf8935e4eaaaf2388ba1f3b499a8d62ad7e8a25 Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Sun, 30 Aug 2026 19:16:06 -0700 Subject: [PATCH 2/4] fix(share): address short-link review feedback --- .github/workflows/test.yml | 1 + .../useSharing.shortUrlLifecycle.test.tsx | 53 +++++++++++++++++++ packages/ui/hooks/useSharing.ts | 24 ++++++--- 3 files changed, 71 insertions(+), 7 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 31f9bfc75..8ce26811b 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -152,6 +152,7 @@ jobs: packages/ui/components/AnnotationPanel.unanchored.test.tsx packages/ui/hooks/useHtmlRefresh.test.tsx packages/ui/hooks/useSharing.contentRevision.test.tsx + packages/ui/hooks/useSharing.shortUrlLifecycle.test.tsx packages/ui/components/AnnotationToolbar.commentOnly.test.tsx packages/ui/components/CommentPopover.quickLookGood.test.tsx packages/editor/actionsLabelMode.test.ts diff --git a/packages/ui/hooks/useSharing.shortUrlLifecycle.test.tsx b/packages/ui/hooks/useSharing.shortUrlLifecycle.test.tsx index 8c84710b8..d9ccfca9d 100644 --- a/packages/ui/hooks/useSharing.shortUrlLifecycle.test.tsx +++ b/packages/ui/hooks/useSharing.shortUrlLifecycle.test.tsx @@ -208,6 +208,59 @@ describe.if(hasDom)('useSharing short URL lifecycle', () => { expect(postCount).toBe(2); }); + test('clears a failed short-link error only after the represented content changes', async () => { + let postCount = 0; + // SAFETY: This boundary fake implements the paste POST exercised by useSharing. + globalThis.fetch = (async (_input, init) => { + if (init?.method === 'POST') { + postCount += 1; + if (postCount === 1) { + return Response.json({ id: 'LocalSuccess' }, { status: 201 }); + } + return Response.json({ error: 'Unavailable' }, { status: 503 }); + } + return Response.json({ error: 'Unexpected request' }, { status: 500 }); + }) as typeof fetch; + + const capture: HarnessCapture = { + result: null, + controls: null, + }; + await mountHarness(0, capture); + + await act(async () => { + capture.controls?.setMarkdown('# Local plan\n\nOriginal content'); + }); + await waitFor(() => Boolean(capture.result?.shareUrl)); + + let initialShortUrl: string | null = null; + await act(async () => { + initialShortUrl = await capture.result?.generateShortUrl() ?? null; + }); + expect(initialShortUrl).toContain('/p/LocalSuccess#key='); + expect(capture.result?.shortShareUrl).toBe(initialShortUrl); + + await act(async () => { + expect(await capture.result?.generateShortUrl()).toBeNull(); + }); + expect(postCount).toBe(2); + expect(capture.result?.shortShareUrl).toBe(''); + expect(capture.result?.shortUrlError).toBe('Short URL service unavailable'); + + await act(async () => { + renderHarness(0, capture); + await new Promise((resolve) => setTimeout(resolve, 0)); + }); + expect(capture.result?.shortUrlError).toBe('Short URL service unavailable'); + + await act(async () => { + capture.controls?.setMarkdown('# Local plan\n\nEdited content'); + }); + expect(capture.result?.shortShareUrl).toBe(''); + expect(capture.result?.shortUrlError).toBe(''); + expect(postCount).toBe(2); + }); + test('invalidates a hydrated HTML short URL when the portable content revision changes', async () => { const payload: SharePayload = { p: '', diff --git a/packages/ui/hooks/useSharing.ts b/packages/ui/hooks/useSharing.ts index a2d68a776..497cf7826 100644 --- a/packages/ui/hooks/useSharing.ts +++ b/packages/ui/hooks/useSharing.ts @@ -80,7 +80,8 @@ type ShortShareUrlLifecycle = | { readonly _tag: 'none' } | { readonly _tag: 'incoming-hydration' } | { readonly _tag: 'generating'; readonly requestContext: object } - | { readonly _tag: 'associated'; readonly requestContext: object }; + | { readonly _tag: 'associated'; readonly requestContext: object } + | { readonly _tag: 'failed'; readonly requestContext: object }; // Share payloads are base64url-encoded deflate output: charset [A-Za-z0-9_-], // realistically >=30 chars, and virtually always mixed-case because deflate @@ -154,9 +155,10 @@ export function useSharing( const pathMatch = window.location.pathname.match(/^\/p\/([A-Za-z0-9]{6,16})$/); if (pathMatch) { const pasteId = pathMatch[1]; - // Capture before the async fetch. React Strict Mode may invoke this - // mount effect twice; the first completion removes /p/ from - // history, but both completions must preserve the original short URL. + // Capture before the async fetch. Concurrent loads (including the + // development Strict Mode replay) can complete after an earlier load + // removes /p/ from history; every completion must preserve the + // original short URL. const incomingShortUrl = window.location.href; // Extract key and optional paste origin from fragment: #key=&paste= @@ -315,6 +317,10 @@ export function useSharing( const lifecycle = shortShareUrlLifecycleRef.current; if (lifecycle._tag === 'incoming-hydration') { if (!shortShareUrl) return; + // Hydration writes fresh annotation and attachment arrays, so this first + // committed request context represents the loaded snapshot. If those + // setters ever preserve identity, replace this consume-on-next-effect + // handoff with an explicit post-hydration signal. shortShareUrlLifecycleRef.current = { _tag: 'associated', requestContext: shareRequestContext, @@ -322,7 +328,11 @@ export function useSharing( return; } if ( - (lifecycle._tag === 'generating' || lifecycle._tag === 'associated') + ( + lifecycle._tag === 'generating' + || lifecycle._tag === 'associated' + || lifecycle._tag === 'failed' + ) && lifecycle.requestContext === shareRequestContext ) { return; @@ -374,14 +384,14 @@ export function useSharing( setShortShareUrl(result.shortUrl); return result.shortUrl; } else { - shortShareUrlLifecycleRef.current = { _tag: 'none' }; + shortShareUrlLifecycleRef.current = { _tag: 'failed', requestContext }; setShortShareUrl(''); setShortUrlError('Short URL service unavailable'); return null; } } catch (e) { if (latestShareRequestContextRef.current !== requestContext) return null; - shortShareUrlLifecycleRef.current = { _tag: 'none' }; + shortShareUrlLifecycleRef.current = { _tag: 'failed', requestContext }; setShortShareUrl(''); setShortUrlError(e instanceof Error ? e.message : 'Failed to generate short URL'); return null; From d570d934ff435820872b4ff3a4df1a87d6bcf270 Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Sun, 30 Aug 2026 19:26:52 -0700 Subject: [PATCH 3/4] test(ci): isolate short-link lifecycle coverage --- .github/workflows/test.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 8ce26811b..8c9b0136b 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -152,7 +152,6 @@ jobs: packages/ui/components/AnnotationPanel.unanchored.test.tsx packages/ui/hooks/useHtmlRefresh.test.tsx packages/ui/hooks/useSharing.contentRevision.test.tsx - packages/ui/hooks/useSharing.shortUrlLifecycle.test.tsx packages/ui/components/AnnotationToolbar.commentOnly.test.tsx packages/ui/components/CommentPopover.quickLookGood.test.tsx packages/editor/actionsLabelMode.test.ts @@ -163,6 +162,9 @@ jobs: packages/ui/components/MathBlock.firstPaint.test.tsx packages/ui/components/DiagramBlock.lazyRetry.test.tsx + - name: Run short URL lifecycle DOM test + run: DOM_TESTS=1 bun test packages/ui/hooks/useSharing.shortUrlLifecycle.test.tsx + opencode-v2: name: OpenCode 2 installed package runs-on: ubuntu-latest From 4d0be150f13a3f292f8e3861cdbeeb75efd27864 Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Sun, 30 Aug 2026 19:30:57 -0700 Subject: [PATCH 4/4] test(ci): isolate registered DOM suites --- .github/workflows/test.yml | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 8c9b0136b..e3e1f06ba 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -75,10 +75,12 @@ jobs: packages/review-editor/components/DiffViewer.headerControls.test.tsx # Seam contracts + the remaining DOM-gated tests. Scoped to the DOM files - # (not the whole ui suite) to keep this process light. + # (not the whole ui suite) to keep this process light. Files get isolated + # globals because theme, storage, and DOM test state must not leak between + # suites when Bun schedules them differently across platforms. - name: Run UI seam-contract + DOM tests run: >- - DOM_TESTS=1 bun test + DOM_TESTS=1 bun test --isolate packages/ui/markdownEditorFidelity.test.tsx packages/ui/annotationDraftPersistence.test.tsx packages/ui/codeAnnotationDraftPersistence.test.tsx @@ -152,6 +154,7 @@ jobs: packages/ui/components/AnnotationPanel.unanchored.test.tsx packages/ui/hooks/useHtmlRefresh.test.tsx packages/ui/hooks/useSharing.contentRevision.test.tsx + packages/ui/hooks/useSharing.shortUrlLifecycle.test.tsx packages/ui/components/AnnotationToolbar.commentOnly.test.tsx packages/ui/components/CommentPopover.quickLookGood.test.tsx packages/editor/actionsLabelMode.test.ts @@ -162,9 +165,6 @@ jobs: packages/ui/components/MathBlock.firstPaint.test.tsx packages/ui/components/DiagramBlock.lazyRetry.test.tsx - - name: Run short URL lifecycle DOM test - run: DOM_TESTS=1 bun test packages/ui/hooks/useSharing.shortUrlLifecycle.test.tsx - opencode-v2: name: OpenCode 2 installed package runs-on: ubuntu-latest