diff --git a/packages/server/call-flow-install-endpoint.test.ts b/packages/server/call-flow-install-endpoint.test.ts index dec2f3399..5e331e218 100644 --- a/packages/server/call-flow-install-endpoint.test.ts +++ b/packages/server/call-flow-install-endpoint.test.ts @@ -6,15 +6,9 @@ import { join } from 'node:path'; import type { CallFlowInstallStage, CallFlowNodePreflight, CallFlowRuntimeInstallResult } from '@plannotator/shared/call-flow'; // PLANNOTATOR_DATA_DIR is only ever changed INSIDE tests (boot() below) and -// restored to its original value after each one. It must never be overridden -// at module-eval time: bun evaluates every test file's module before running -// tests in one shared process, and Pi's generated/storage.ts caches its data -// dir at import time. A module-eval override here makes storage's cached dir -// and later files' live getPlannotatorDataDir() calls disagree, which is -// exactly the Pi annotate-history / durable-submit CI failure this comment -// guards against. Config writes made by these tests target whatever dir the -// process's config module froze at first import; the snapshot/restore in -// afterAll below keeps those writes from leaking into a real config.json. +// restored after each one. Module-eval overrides would leak into other test +// files because Bun runs the suite in one shared process. The config +// snapshot/restore in afterAll also protects against shared config state. const originalDataDir = process.env.PLANNOTATOR_DATA_DIR; const originalPort = process.env.PLANNOTATOR_PORT; const originalPath = process.env.PATH; diff --git a/packages/server/storage.test.ts b/packages/server/storage.test.ts index be005caa7..8bdee91f2 100644 --- a/packages/server/storage.test.ts +++ b/packages/server/storage.test.ts @@ -174,3 +174,39 @@ describe("listVersions", () => { expect(versions[0].timestamp).toBeTruthy(); }); }); + +describe("PLANNOTATOR_DATA_DIR", () => { + test("isolates plan and history data when the data directory changes after import", () => { + const savedDataDir = process.env.PLANNOTATOR_DATA_DIR; + const firstDir = makeTempDir(); + const secondDir = makeTempDir(); + const project = "data-dir-project"; + const slug = "data-dir-plan"; + + try { + process.env.PLANNOTATOR_DATA_DIR = firstDir; + savePlan(slug, "# First plan"); + saveToHistory(project, slug, "# First version"); + expect(readFileSync(join(firstDir, "plans", `${slug}.md`), "utf-8")).toBe("# First plan"); + expect(getPlanVersion(project, slug, 1)).toBe("# First version"); + expect(getVersionCount(project, slug)).toBe(1); + + process.env.PLANNOTATOR_DATA_DIR = secondDir; + expect(getPlanVersion(project, slug, 1)).toBeNull(); + expect(getVersionCount(project, slug)).toBe(0); + savePlan(slug, "# Second plan"); + saveToHistory(project, slug, "# Second version"); + expect(readFileSync(join(secondDir, "plans", `${slug}.md`), "utf-8")).toBe("# Second plan"); + expect(getPlanVersion(project, slug, 1)).toBe("# Second version"); + expect(getVersionCount(project, slug)).toBe(1); + + process.env.PLANNOTATOR_DATA_DIR = firstDir; + expect(readFileSync(join(firstDir, "plans", `${slug}.md`), "utf-8")).toBe("# First plan"); + expect(getPlanVersion(project, slug, 1)).toBe("# First version"); + expect(getVersionCount(project, slug)).toBe(1); + } finally { + if (savedDataDir === undefined) delete process.env.PLANNOTATOR_DATA_DIR; + else process.env.PLANNOTATOR_DATA_DIR = savedDataDir; + } + }); +}); diff --git a/packages/shared/storage.ts b/packages/shared/storage.ts index cc4e12f66..5c3662043 100644 --- a/packages/shared/storage.ts +++ b/packages/shared/storage.ts @@ -13,8 +13,6 @@ import { sanitizeTag } from "./project"; import { resolveUserPath } from "./resolve-file"; import { getPlannotatorDataDir } from "./data-dir"; -const DATA_DIR = getPlannotatorDataDir(); - /** * Get the plan storage directory, creating it if needed. * Cross-platform: uses os.homedir() for Windows/macOS/Linux compatibility. @@ -26,7 +24,7 @@ export function getPlanDir(customPath?: string | null): string { if (customPath?.trim()) { planDir = resolveUserPath(customPath); } else { - planDir = join(DATA_DIR, "plans"); + planDir = join(getPlannotatorDataDir(), "plans"); } mkdirSync(planDir, { recursive: true }); @@ -195,7 +193,7 @@ export function readArchivedPlan(filename: string, customPath?: string | null): * Not affected by the customPath setting (that only affects decision saves). */ export function getHistoryDir(project: string, slug: string): string { - const historyDir = join(DATA_DIR, "history", project, slug); + const historyDir = join(getPlannotatorDataDir(), "history", project, slug); mkdirSync(historyDir, { recursive: true }); return historyDir; } @@ -294,7 +292,7 @@ export function getPlanVersion( slug: string, version: number ): string | null { - const historyDir = join(DATA_DIR, "history", project, slug); + const historyDir = join(getPlannotatorDataDir(), "history", project, slug); const fileName = `${String(version).padStart(3, "0")}.md`; const filePath = join(historyDir, fileName); @@ -314,7 +312,7 @@ export function getPlanVersionPath( slug: string, version: number ): string | null { - const historyDir = join(DATA_DIR, "history", project, slug); + const historyDir = join(getPlannotatorDataDir(), "history", project, slug); const fileName = `${String(version).padStart(3, "0")}.md`; const filePath = join(historyDir, fileName); return existsSync(filePath) ? filePath : null; @@ -325,7 +323,7 @@ export function getPlanVersionPath( * Returns 0 if the directory doesn't exist. */ export function getVersionCount(project: string, slug: string): number { - const historyDir = join(DATA_DIR, "history", project, slug); + const historyDir = join(getPlannotatorDataDir(), "history", project, slug); try { const entries = readdirSync(historyDir); return entries.filter((e) => /^\d+\.md$/.test(e)).length; @@ -342,7 +340,7 @@ export function listVersions( project: string, slug: string ): Array<{ version: number; timestamp: string }> { - const historyDir = join(DATA_DIR, "history", project, slug); + const historyDir = join(getPlannotatorDataDir(), "history", project, slug); try { const entries = readdirSync(historyDir); const versions: Array<{ version: number; timestamp: string }> = []; @@ -372,7 +370,7 @@ export function listVersions( export function listProjectPlans( project: string ): Array<{ slug: string; versions: number; lastModified: string }> { - const projectDir = join(DATA_DIR, "history", project); + const projectDir = join(getPlannotatorDataDir(), "history", project); try { const entries = readdirSync(projectDir, { withFileTypes: true }); const plans: Array<{ slug: string; versions: number; lastModified: string }> = []; diff --git a/packages/ui/components/ExportModal.tsx b/packages/ui/components/ExportModal.tsx index 33e7ae3eb..3ee839666 100644 --- a/packages/ui/components/ExportModal.tsx +++ b/packages/ui/components/ExportModal.tsx @@ -136,7 +136,7 @@ export const ExportModal: React.FC = ({ await handleCopy(wrapCopiedAnnotations(annotationsOutput), 'annotations'); }; - // Whether the hash URL is large enough to warrant a short URL option + // Warn when the hash URL may be too long for messaging apps const urlIsLarge = shareUrl.length > 2048; // Hash-based sharing unavailable (e.g. HTML render mode) — show only short link const hashUnavailable = !shareUrl && !!onGenerateShortUrl; @@ -327,9 +327,13 @@ export const ExportModal: React.FC = ({ Generating short link... - ) : (urlIsLarge || hashUnavailable) && onGenerateShortUrl ? ( -
- {!hashUnavailable && ( + ) : onGenerateShortUrl ? ( +
+ {urlIsLarge && (

This URL may be too long for some messaging apps.

diff --git a/packages/ui/hooks/useSharing.shortUrlLifecycle.test.tsx b/packages/ui/hooks/useSharing.shortUrlLifecycle.test.tsx index d9ccfca9d..7d3e123e6 100644 --- a/packages/ui/hooks/useSharing.shortUrlLifecycle.test.tsx +++ b/packages/ui/hooks/useSharing.shortUrlLifecycle.test.tsx @@ -1,18 +1,30 @@ import React, { useState } from 'react'; -import { afterEach, describe, expect, test } from 'bun:test'; +import { afterEach, beforeEach, 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 { decrypt, encrypt } from '@plannotator/core/crypto'; import { useSharing } from './useSharing'; import { AnnotationType, type Annotation, type ImageAttachment } from '../types'; -import type { SharePayload } from '../utils/sharing'; +import { fromShareable, loadFromPasteId, type SharePayload } from '../utils/sharing'; +import { ExportModal } from '../components/ExportModal'; const hasDom = typeof document !== 'undefined'; -const originalFetch = globalThis.fetch; +let originalFetch: typeof fetch; +let originalUrl: string; +let originalHistoryState: unknown; let root: Root | null = null; let host: HTMLDivElement | null = null; +beforeEach(() => { + if (!hasDom) return; + originalFetch = globalThis.fetch; + originalUrl = window.location.href; + originalHistoryState = window.history.state; + window.location.href = 'http://localhost/'; + window.history.replaceState({}, '', '/'); +}); + afterEach(() => { if (!hasDom) return; act(() => root?.unmount()); @@ -20,7 +32,8 @@ afterEach(() => { host?.remove(); host = null; globalThis.fetch = originalFetch; - window.history.replaceState({}, '', '/'); + window.location.href = originalUrl; + window.history.replaceState(originalHistoryState, '', originalUrl); }); type SharingResult = ReturnType; @@ -36,14 +49,21 @@ interface HarnessCapture { controls: SharingControls | null; } +interface ModalOptions { + sharingEnabled?: boolean; + shortLinksSupported?: boolean; +} + function Harness({ contentRevision, onResult, onControls, + modal, }: { contentRevision: number; onResult: (result: SharingResult) => void; onControls: (controls: SharingControls) => void; + modal?: ModalOptions; }) { const [markdown, setMarkdown] = useState(''); const [annotations, setAnnotations] = useState([]); @@ -71,7 +91,22 @@ function Harness({ onResult(result); onControls({ setMarkdown, setAnnotations, setAttachments }); - return null; + return modal ? ( + {}} + shareUrl={result.shareUrl} + shareUrlSize={result.shareUrlSize} + shortShareUrl={result.shortShareUrl} + isGeneratingShortUrl={result.isGeneratingShortUrl} + shortUrlError={result.shortUrlError} + onGenerateShortUrl={modal.shortLinksSupported === false ? undefined : result.generateShortUrl} + sharingEnabled={modal.sharingEnabled} + annotationsOutput="" + annotationCount={annotations.length} + markdown={markdown} + /> + ) : null; } async function waitFor(condition: () => boolean): Promise { @@ -83,6 +118,36 @@ async function waitFor(condition: () => boolean): Promise { expect(condition()).toBe(true); } +async function waitForSharedPayload( + getShareUrl: () => string | undefined, + predicate: (payload: SharePayload) => boolean, +): Promise { + let lastPayload: SharePayload | null = null; + for (let attempt = 0; attempt < 50; attempt += 1) { + const rawUrl = getShareUrl(); + const hashIndex = rawUrl ? rawUrl.indexOf('#') : -1; + if (hashIndex !== -1 && rawUrl) { + const hash = rawUrl.slice(hashIndex + 1); + if (hash) { + try { + const payload = (await decompress(hash)) as SharePayload; + lastPayload = payload; + if (predicate(payload)) { + return payload; + } + } catch { + // Decompress can fail during mid-update or malformed hash + } + } + } + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 0)); + }); + } + expect(lastPayload ? predicate(lastPayload) : false).toBe(true); + return lastPayload!; +} + async function installIncomingPaste( payload: SharePayload, pasteId: string, @@ -99,29 +164,200 @@ async function installIncomingPaste( async function mountHarness( contentRevision: number, capture: HarnessCapture, + modal?: ModalOptions, ): Promise { host = document.createElement('div'); document.body.appendChild(host); root = createRoot(host); await act(async () => { - renderHarness(contentRevision, capture); + renderHarness(contentRevision, capture, modal); await new Promise((resolve) => setTimeout(resolve, 0)); }); } -function renderHarness(contentRevision: number, capture: HarnessCapture): void { +function renderHarness(contentRevision: number, capture: HarnessCapture, modal?: ModalOptions): void { root?.render( { capture.result = result; }} onControls={(controls) => { capture.controls = controls; }} + modal={modal} /> , ); } +function installPasteService(initial: Record = {}) { + const pastes = new Map(Object.entries(initial)); + const uploads: string[] = []; + const rejectedMutations: string[] = []; + // SAFETY: Only the in-memory paste endpoint is implemented, with real Request/Response bodies. + globalThis.fetch = (async (input, init) => { + const request = new Request(input, init); + const url = new URL(request.url); + if (url.origin === 'https://paste.example.test' && url.pathname === '/api/paste' && request.method === 'POST') { + const body = await request.json() as { data: string }; + const id = `Local00${uploads.length + 1}`; + uploads.push(body.data); + pastes.set(id, body.data); + return Response.json({ id }, { status: 201 }); + } + if (request.method !== 'GET') { + rejectedMutations.push(`${request.method} ${request.url}`); + return Response.json({ error: 'Immutable paste' }, { status: 405 }); + } + const id = url.pathname.match(/^\/api\/paste\/([A-Za-z0-9]+)$/)?.[1]; + const data = url.origin === 'https://paste.example.test' && id ? pastes.get(id) : undefined; + return data === undefined + ? Response.json({ error: 'Unknown paste' }, { status: 404 }) + : Response.json({ data }); + }) as typeof fetch; + return { pastes, uploads, rejectedMutations }; +} + +function createShortLinkButton(): HTMLButtonElement | undefined { + // Deliberately select the user-facing action, not the surrounding explanatory copy. + return Array.from(host?.querySelectorAll('button') ?? []) + .find((button) => button.textContent?.trim() === 'Create short link'); +} + +function displayedShortUrl(): string { + return host?.querySelector('input[readonly]')?.value ?? ''; +} + describe.if(hasDom)('useSharing short URL lifecycle', () => { + test('offers an explicit replacement for an edited small incoming encrypted snapshot', async () => { + const originalPayload: SharePayload = { + p: '# Shared plan\n\nOriginal document', + a: [['C', 'Original document', 'Initial feedback', null]], + }; + const incoming = await installIncomingPaste(originalPayload, 'AbCd1234'); + const { data: originalCiphertext } = await incoming.getPasteResponse.json() as { data: string }; + const service = installPasteService({ AbCd1234: originalCiphertext }); + const capture: HarnessCapture = { result: null, controls: null }; + const modal: ModalOptions = {}; + await mountHarness(0, capture, modal); + await waitFor(() => capture.result?.isLoadingShared === false); + await waitForSharedPayload(() => capture.result?.shareUrl, (payload) => payload.p === originalPayload.p); + + expect(displayedShortUrl()).toBe(incoming.incomingUrl); + expect(service.uploads).toEqual([]); + const hydratedShareUrl = capture.result!.shareUrl; + expect(hydratedShareUrl.length).toBeLessThan(2048); + await act(async () => { + renderHarness(0, capture, modal); + }); + expect(displayedShortUrl()).toBe(incoming.incomingUrl); + expect(service.uploads).toEqual([]); + + const editedMarkdown = '# Shared plan\n\nEdited document'; + function expectEditedDocument(payload: SharePayload): void { + expect(payload.p).toBe(editedMarkdown); + expect(fromShareable(payload.a)).toEqual([ + expect.objectContaining({ + type: AnnotationType.COMMENT, + originalText: 'Original document', + text: 'Updated feedback', + }), + ]); + } + await act(async () => { + capture.controls!.setMarkdown(editedMarkdown); + capture.controls!.setAnnotations((current) => current.map((annotation) => ({ + ...annotation, + text: 'Updated feedback', + }))); + }); + expect(displayedShortUrl()).toBe(''); + await waitForSharedPayload(() => capture.result?.shareUrl, (payload) => payload.p === editedMarkdown); + const fullUrl = host!.querySelector('textarea')!.value; + expect(fullUrl.length).toBeLessThan(2048); + expectEditedDocument(await decompress(new URL(fullUrl).hash.slice(1)) as SharePayload); + expect(createShortLinkButton()).toBeDefined(); + expect(service.uploads).toEqual([]); + + await act(async () => { + renderHarness(0, capture, modal); + }); + expect(service.uploads).toEqual([]); + await act(async () => { + createShortLinkButton()!.click(); + }); + await waitFor(() => displayedShortUrl() !== ''); + const newUrl = new URL(displayedShortUrl()); + expect(newUrl.href).not.toBe(incoming.incomingUrl); + expect(newUrl.pathname).toBe('/p/Local001'); + expect(service.uploads).toHaveLength(1); + const newCiphertext = service.pastes.get('Local001')!; + expect(newCiphertext).not.toBe(originalCiphertext); + const newKey = new URLSearchParams(newUrl.hash.slice(1)).get('key')!; + expectEditedDocument(await decompress(await decrypt(newCiphertext, newKey)) as SharePayload); + + const originalKey = new URLSearchParams(new URL(incoming.incomingUrl).hash.slice(1)).get('key')!; + expect(await loadFromPasteId('AbCd1234', 'https://paste.example.test', originalKey)).toEqual(originalPayload); + expect(service.pastes.get('AbCd1234')).toBe(originalCiphertext); + expect(service.rejectedMutations).toEqual([]); + expect(service.uploads).toHaveLength(1); + }); + + test('offers short-link creation for a fresh small plan without uploading until clicked', async () => { + const service = installPasteService(); + const capture: HarnessCapture = { result: null, controls: null }; + const modal: ModalOptions = {}; + await mountHarness(0, capture, modal); + await waitFor(() => capture.result?.isLoadingShared === false && Boolean(capture.result?.shareUrl)); + const emptyPlanUrl = capture.result!.shareUrl; + await act(async () => { + capture.controls!.setMarkdown('# Fresh small plan'); + }); + await waitFor(() => Boolean(capture.result?.shareUrl) && capture.result?.shareUrl !== emptyPlanUrl); + expect(capture.result?.isSharedSession).toBe(false); + expect(displayedShortUrl()).toBe(''); + const fullUrl = host!.querySelector('textarea')!.value; + expect(fullUrl.length).toBeLessThan(2048); + expect(await decompress(new URL(fullUrl).hash.slice(1))).toEqual({ p: '# Fresh small plan', a: [] }); + expect(createShortLinkButton()).toBeDefined(); + await act(async () => { + renderHarness(0, capture, modal); + }); + expect(service.uploads).toEqual([]); + + await act(async () => { + createShortLinkButton()!.click(); + }); + await waitFor(() => displayedShortUrl() !== ''); + expect(service.uploads).toHaveLength(1); + const url = new URL(displayedShortUrl()); + const key = new URLSearchParams(url.hash.slice(1)).get('key')!; + expect(await loadFromPasteId(url.pathname.split('/').pop()!, 'https://paste.example.test', key)) + .toEqual({ p: '# Fresh small plan', a: [] }); + expect(service.rejectedMutations).toEqual([]); + }); + + test('does not expose creation when sharing is disabled or short links are unsupported', async () => { + const service = installPasteService(); + const capture: HarnessCapture = { result: null, controls: null }; + await mountHarness(0, capture, { sharingEnabled: false }); + await waitFor(() => capture.result?.isLoadingShared === false && Boolean(capture.result?.shareUrl)); + const emptyPlanUrl = capture.result!.shareUrl; + await act(async () => { + capture.controls!.setMarkdown('# Private plan'); + }); + await waitFor(() => Boolean(capture.result?.shareUrl) && capture.result?.shareUrl !== emptyPlanUrl); + expect(createShortLinkButton()).toBeUndefined(); + + await act(async () => { + renderHarness(0, capture, { shortLinksSupported: false }); + }); + // Full URL sharing still works on a surface that has no paste-generation callback. + const fullUrl = host!.querySelector('textarea')!.value; + expect(await decompress(new URL(fullUrl).hash.slice(1))).toEqual({ p: '# Private plan', a: [] }); + expect(createShortLinkButton()).toBeUndefined(); + expect(service.uploads).toEqual([]); + expect(service.rejectedMutations).toEqual([]); + }); + 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', @@ -150,7 +386,10 @@ describe.if(hasDom)('useSharing short URL lifecycle', () => { expect(capture.result?.shortShareUrl).toBe(incoming.incomingUrl); expect(capture.result?.isSharedSession).toBe(true); - await waitFor(() => Boolean(capture.result?.shareUrl)); + await waitForSharedPayload( + () => capture.result?.shareUrl, + (p) => p.p === payload.p && p.a.length === 1, + ); const hydratedShareUrl = capture.result?.shareUrl ?? ''; await act(async () => { @@ -168,10 +407,10 @@ describe.if(hasDom)('useSharing short URL lifecycle', () => { 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; + const annotatedSharePayload = await waitForSharedPayload( + () => capture.result?.shareUrl, + (p) => p.a.length === 2, + ); expect(annotatedSharePayload.a).toHaveLength(2); let firstLocalUrl: string | null = null;