From 5cb3dca54d11933718ae90a1997a33c7b9bbf5e4 Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Fri, 31 Jul 2026 12:55:27 -0700 Subject: [PATCH 1/2] fix(annotate): enforce archive read-only surfaces --- .github/workflows/test.yml | 2 + apps/pi-extension/server/serverPlan.ts | 6 + apps/pi-extension/vendor.sh | 2 +- packages/editor/App.archiveReadOnly.test.tsx | 169 ++++++++++++++++++ packages/editor/App.tsx | 53 ++++-- packages/editor/actionsLabelMode.test.ts | 78 ++++++++ packages/editor/actionsLabelMode.ts | 24 +++ packages/server/api-404-guard.test.ts | 52 ++++++ packages/server/index.ts | 5 + packages/shared/archive-mode.ts | 16 ++ packages/shared/package.json | 3 +- .../components/AnnotationPanel.props.test.tsx | 19 ++ packages/ui/components/AnnotationPanel.tsx | 6 +- .../ui/components/Viewer.consumer.test.tsx | 58 ++++++ .../ui/components/html-viewer/HtmlViewer.tsx | 86 +++++---- .../html-viewer/useHtmlAnnotation.ts | 24 +++ 16 files changed, 543 insertions(+), 60 deletions(-) create mode 100644 packages/editor/App.archiveReadOnly.test.tsx create mode 100644 packages/editor/actionsLabelMode.test.ts create mode 100644 packages/editor/actionsLabelMode.ts create mode 100644 packages/shared/archive-mode.ts diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 59650a3df..f2df339cc 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -69,6 +69,8 @@ jobs: packages/ui/hooks/useLinkedDoc.test.tsx packages/ui/components/DocBadges.test.tsx packages/editor/planDiffAutoExit.test.tsx + packages/editor/App.archiveReadOnly.test.tsx + packages/editor/actionsLabelMode.test.ts pi-extension-ai-runtime-windows: # Exercises the Pi extension's Node/jiti server mirror on Windows with an diff --git a/apps/pi-extension/server/serverPlan.ts b/apps/pi-extension/server/serverPlan.ts index 399618739..c299b7507 100644 --- a/apps/pi-extension/server/serverPlan.ts +++ b/apps/pi-extension/server/serverPlan.ts @@ -53,6 +53,7 @@ import { } from "./reference.ts"; import { handleFileBrowserStreamRequest } from "./file-browser-watch.ts"; import { warmFileListCache } from "../generated/resolve-file.ts"; +import { isArchiveDocumentMutation } from "../generated/archive-mode.ts"; export interface PlanReviewDecision { approved: boolean; @@ -169,6 +170,11 @@ export async function startPlanReviewServer(options: { if (url.pathname === "/api/done" && req.method === "POST") { resolveDone?.(); json(res, { ok: true }); + } else if ( + options.mode === "archive" && + isArchiveDocumentMutation(req.method ?? "GET", url.pathname) + ) { + json(res, { error: "Archive is read-only" }, 403); } else if (url.pathname === "/api/archive/plans" && req.method === "GET") { const customPath = url.searchParams.get("customPath") || undefined; if (!cachedArchivePlans) diff --git a/apps/pi-extension/vendor.sh b/apps/pi-extension/vendor.sh index 14e937e46..202d5ee10 100755 --- a/apps/pi-extension/vendor.sh +++ b/apps/pi-extension/vendor.sh @@ -29,7 +29,7 @@ for f in config-types storage-types workspace-status-types; do done # Everything else in the original flat list stays sourced from packages/shared. -for f in prompts review-core diff-paths cli-pagination jj-core gitbutler-core vcs-core review-args draft annotate-history pr-types pr-context-live pr-artifact-document pr-provider pr-stack pr-github pr-gitlab checklist integrations-common repo reference-common resolve-file annotate-reference-roots-node worktree worktree-pool html-to-markdown html-diff html-assets html-assets-node url-to-markdown tour annotate-args at-reference review-workspace-node review-workspace pfm-reminder improvement-hooks code-nav data-dir semantic-diff-types semantic-diff single-flight source-save-node review-profiles guide guide-store commit-avatars commit-history port-range annotate-client-lease annotate-decision; do +for f in prompts review-core diff-paths cli-pagination jj-core gitbutler-core vcs-core review-args draft annotate-history pr-types pr-context-live pr-artifact-document pr-provider pr-stack pr-github pr-gitlab checklist integrations-common repo reference-common resolve-file annotate-reference-roots-node worktree worktree-pool html-to-markdown html-diff html-assets html-assets-node url-to-markdown tour annotate-args at-reference review-workspace-node review-workspace pfm-reminder improvement-hooks code-nav data-dir semantic-diff-types semantic-diff single-flight source-save-node review-profiles guide guide-store commit-avatars commit-history port-range annotate-client-lease annotate-decision archive-mode; do src="../../packages/shared/$f.ts" printf '// @generated — DO NOT EDIT. Source: packages/shared/%s.ts\n' "$f" | cat - "$src" > "generated/$f.ts" done diff --git a/packages/editor/App.archiveReadOnly.test.tsx b/packages/editor/App.archiveReadOnly.test.tsx new file mode 100644 index 000000000..1be22fafd --- /dev/null +++ b/packages/editor/App.archiveReadOnly.test.tsx @@ -0,0 +1,169 @@ +import { afterEach, describe, expect, test } from "bun:test"; +import React, { act } from "react"; +import { createRoot, type Root } from "react-dom/client"; + +const hasDom = typeof document !== "undefined"; + +if (hasDom) { + document.cookie = "plannotator-look-feel-announcement-seen=2; path=/"; + document.cookie = "plannotator-vim-mode-announcement-seen=2; path=/"; + document.cookie = "plannotator-plan-ai-announcement-seen=1; path=/"; +} + +const appModule = hasDom ? await import("./App") : null; +const App = appModule?.default as typeof import("./App")["default"]; +const originalFetch = globalThis.fetch; +const originalEventSource = globalThis.EventSource; + +interface PlanResponse { + readonly plan: string; + readonly origin: "codex"; + readonly mode: "archive" | "annotate"; + readonly filePath?: string; + readonly archivePlans?: readonly [{ + readonly filename: string; + readonly status: "approved"; + readonly timestamp: string; + readonly title: string; + }]; + readonly sharingEnabled: false; + readonly serverConfig: Record; +} + +class SilentEventSource { + static readonly CONNECTING = 0; + static readonly OPEN = 1; + static readonly CLOSED = 2; + + readonly CONNECTING = 0; + readonly OPEN = 1; + readonly CLOSED = 2; + readonly readyState = SilentEventSource.OPEN; + readonly url: string; + readonly withCredentials = false; + onerror: ((event: Event) => void) | null = null; + onmessage: ((event: MessageEvent) => void) | null = null; + onopen: ((event: Event) => void) | null = null; + + constructor(url: string | URL) { + this.url = String(url); + } + + addEventListener(): void {} + close(): void {} + dispatchEvent(): boolean { return true; } + removeEventListener(): void {} +} + +let root: Root | null = null; +let host: HTMLElement | null = null; +let requestedRoutes: string[] = []; + +function responseFor(planResponse: PlanResponse): typeof fetch { + return async (input, init) => { + const rawUrl = input instanceof Request ? input.url : String(input); + const method = input instanceof Request ? input.method : init?.method ?? "GET"; + if (rawUrl.startsWith("https://api.github.com/")) { + return new Response(null, { status: 404 }); + } + + const url = new URL(rawUrl, "http://localhost"); + requestedRoutes.push(`${method} ${url.pathname}`); + if (url.pathname === "/api/plan") return Response.json(planResponse); + if (url.pathname === "/api/archive/plans") { + return Response.json({ plans: planResponse.archivePlans ?? [] }); + } + if (url.pathname === "/api/archive/plan") { + return Response.json({ markdown: planResponse.plan, filepath: "saved.md" }); + } + if (url.pathname === "/api/ai/capabilities") { + return Response.json({ available: false, providers: [] }); + } + if (url.pathname === "/api/open-in/apps") { + return Response.json({ + available: true, + apps: [{ id: "reveal", label: "Finder", kind: "file-manager", icon: "finder" }], + }); + } + if (url.pathname === "/api/draft") { + return Response.json({ error: "Not found" }, { status: 404 }); + } + return Response.json({}); + }; +} + +async function mountApp(planResponse: PlanResponse): Promise { + requestedRoutes = []; + globalThis.fetch = responseFor(planResponse); + // SAFETY: the App only uses EventSource's constructor, handlers, and close; + // this test double implements those browser-facing members without I/O. + globalThis.EventSource = SilentEventSource as unknown as typeof EventSource; + host = document.createElement("div"); + document.body.appendChild(host); + root = createRoot(host); + await act(async () => { + root?.render(); + }); + + for (let attempt = 0; attempt < 20 && !document.body.textContent?.includes(planResponse.plan.slice(2)); attempt += 1) { + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 0)); + }); + } +} + +afterEach(async () => { + if (root) await act(async () => root?.unmount()); + root = null; + host?.remove(); + host = null; + globalThis.fetch = originalFetch; + globalThis.EventSource = originalEventSource; + if (hasDom) document.body.replaceChildren(); +}); + +describe.if(hasDom)("App document permissions", () => { + test("standalone archive renders Markdown without mutation entry points", async () => { + await mountApp({ + plan: "# Archived document", + origin: "codex", + mode: "archive", + archivePlans: [{ + filename: "saved.md", + status: "approved", + timestamp: "2026-07-31T00:00:00.000Z", + title: "Archived document", + }], + sharingEnabled: false, + serverConfig: {}, + }); + + expect(document.body.textContent).toContain("Archived document"); + expect(document.querySelector('button[title="Add global comment"]')).toBeNull(); + expect(document.querySelector('button[title="Attachments"]')).toBeNull(); + + await act(async () => { + window.dispatchEvent(new KeyboardEvent("keydown", { + key: "Enter", + metaKey: true, + })); + }); + expect(requestedRoutes).not.toContain("POST /api/approve"); + expect(requestedRoutes).not.toContain("POST /api/deny"); + }); + + test("normal annotate remains writable", async () => { + await mountApp({ + plan: "# Writable document", + origin: "codex", + mode: "annotate", + filePath: "/tmp/writable.md", + sharingEnabled: false, + serverConfig: {}, + }); + + expect(document.body.textContent).toContain("Writable document"); + expect(document.querySelector('button[title="Add global comment"]')).not.toBeNull(); + expect(document.querySelector('button[title="Attachments"]')).not.toBeNull(); + }); +}); diff --git a/packages/editor/App.tsx b/packages/editor/App.tsx index 0bb2b781c..4ca5186af 100644 --- a/packages/editor/App.tsx +++ b/packages/editor/App.tsx @@ -101,6 +101,7 @@ import { type SourceSaveResponse, } from '@plannotator/shared/source-save'; import type { AgentTerminalCapability } from '@plannotator/shared/agent-terminal'; +import { observeActionsLabelMode } from './actionsLabelMode'; // Demo content toggle. Default: the original Real-time Collaboration plan. // Opt-in diff-engine stress test: `VITE_DIFF_DEMO=1 bun run dev:hook` swaps // in the 20-case Auth Service Refactor test plan. dev-mock-api.ts reads the @@ -915,6 +916,7 @@ const App: React.FC = () => { markdown, viewerRef, linkedDocHook, setMarkdown, setAnnotations, setSelectedAnnotationId, setSubmitted, }); + const documentReadOnly = archive.archiveMode; const canUseWideMode = useMemo(() => canUseAnnotateWideMode({ archiveMode: archive.archiveMode, @@ -1340,7 +1342,9 @@ const App: React.FC = () => { const activeSection = useActiveSection(containerRef, headingCount, scrollViewport); const { editorAnnotations, deleteEditorAnnotation } = useEditorAnnotations(); - const { externalAnnotations, updateExternalAnnotation, deleteExternalAnnotation } = useExternalAnnotations({ enabled: isApiMode && !goalSetupMode }); + const { externalAnnotations, updateExternalAnnotation, deleteExternalAnnotation } = useExternalAnnotations({ + enabled: isApiMode && !goalSetupMode && !documentReadOnly, + }); // Drive DOM highlights for SSE-delivered external annotations. Disabled // while a linked doc overlay is open (Viewer DOM is hidden) and while the @@ -1557,15 +1561,9 @@ const App: React.FC = () => { const el = planAreaRef.current; if (!el) return; - const bucket = (w: number): ActionsLabelMode => - w >= 800 ? 'full' : w >= 680 ? 'short' : 'icon'; - setActionsLabelMode(bucket(el.getBoundingClientRect().width)); - const ro = new ResizeObserver(([entry]) => { - const next = bucket(entry.contentRect.width); + return observeActionsLabelMode(el, (next) => { setActionsLabelMode((prev) => (prev === next ? prev : next)); }); - ro.observe(el); - return () => ro.disconnect(); }, [isLoading, isSharedSession]); // The user's current direct-edit text: the open editor buffer, else the @@ -1599,7 +1597,7 @@ const App: React.FC = () => { getEditedMarkdown: getDraftEditedMarkdown, getEditedDocuments: editableDocuments.getDraftDocuments, getSavedFileChanges: editableDocuments.getDraftSavedFileChanges, - isApiMode: isApiMode && !goalSetupMode, + isApiMode: isApiMode && !goalSetupMode && !documentReadOnly, isSharedSession, // isSubmitting counts: a save firing while approve/deny is in flight can // land after the server's draft delete and ghost a "Draft Recovered" @@ -2624,6 +2622,7 @@ const App: React.FC = () => { // Global paste listener for image attachments useEffect(() => { + if (documentReadOnly) return; const handlePaste = (e: ClipboardEvent) => { const items = e.clipboardData?.items; if (!items) return; @@ -2645,11 +2644,11 @@ const App: React.FC = () => { document.addEventListener('paste', handlePaste); return () => document.removeEventListener('paste', handlePaste); - }, [globalAttachments]); + }, [documentReadOnly, globalAttachments]); // Handle paste annotator accept — name comes from ImageAnnotator const handlePasteAnnotatorAccept = async (blob: Blob, hasDrawings: boolean, name: string) => { - if (!pendingPasteImage) return; + if (documentReadOnly || !pendingPasteImage) return; try { const formData = new FormData(); @@ -3113,6 +3112,9 @@ const App: React.FC = () => { // Don't intercept in demo/share mode (no API) if (!isApiMode) return; + // Standalone archive is navigable but has no review decision to submit. + if (documentReadOnly) return; + // While the markdown editor is open, submit shortcuts belong to editing, // not the review session. if (isEditingMarkdown) return; @@ -3177,13 +3179,14 @@ const App: React.FC = () => { }, [ showExport, showImport, showFeedbackPrompt, showClaudeCodeWarning, showSourceFileEditWarning, showExitWarning, showApproveWithNotesConfirmation, showAgentWarning, showPermissionModeSetup, pendingPasteImage, - submitted, isSubmitting, isExiting, goalSetupAction.isSubmitting, isApiMode, isEditingMarkdown, linkedDocHook.isActive, annotations.length, codeAnnotations.length, externalAnnotations.length, annotateMode, + submitted, isSubmitting, isExiting, goalSetupAction.isSubmitting, isApiMode, documentReadOnly, isEditingMarkdown, linkedDocHook.isActive, annotations.length, codeAnnotations.length, externalAnnotations.length, annotateMode, gate, approvalNotesSupported, hasFeedbackToSend, goalSetupMode, goalSetupAction.canSubmit, isAgentTerminalReady, annotateSource, origin, getAgentWarning, maybeConfirmUnsavedSourceFileEdits, ]); const handleAddAnnotation = (ann: Annotation) => { + if (documentReadOnly) return; setAnnotations(prev => [...prev, ann]); setSelectedAnnotationId(ann.id); setSelectedCodeAnnotationId(null); @@ -3197,6 +3200,7 @@ const App: React.FC = () => { }, [isMobile, wideModeType]); const handleAddCodeAnnotation = React.useCallback((input: CodeFileAnnotationInput) => { + if (documentReadOnly) return; const annotation: CodeAnnotation = { id: generateId('code-ann'), type: 'comment', @@ -3214,7 +3218,7 @@ const App: React.FC = () => { setCodeAnnotations(prev => [...prev, annotation]); setSelectedAnnotationId(null); setSelectedCodeAnnotationId(annotation.id); - }, []); + }, [documentReadOnly]); // The code popout is full-viewport modal — the annotation panel is behind it. // This handler only fires when the popout is closed (sidebar visible), so @@ -3229,16 +3233,19 @@ const App: React.FC = () => { }, [codeAnnotations, codeFilePopout.open, isMobile, wideModeType]); const handleDeleteCodeAnnotation = React.useCallback((id: string) => { + if (documentReadOnly) return; setCodeAnnotations(prev => prev.filter(a => a.id !== id)); if (selectedCodeAnnotationId === id) setSelectedCodeAnnotationId(null); - }, [selectedCodeAnnotationId]); + }, [documentReadOnly, selectedCodeAnnotationId]); const handleEditCodeAnnotation = React.useCallback((id: string, updates: Partial) => { + if (documentReadOnly) return; setCodeAnnotations(prev => prev.map(a => a.id === id ? { ...a, ...updates } : a)); - }, []); + }, [documentReadOnly]); // Core annotation removal — highlight cleanup + state filter + selection clear const removeAnnotation = (id: string) => { + if (documentReadOnly) return; viewerRef.current?.removeHighlight(id); setAnnotations(prev => prev.filter(a => a.id !== id)); if (selectedAnnotationId === id) setSelectedAnnotationId(null); @@ -3253,6 +3260,7 @@ const App: React.FC = () => { }); const handleDeleteAnnotation = (id: string) => { + if (documentReadOnly) return; const ann = allAnnotations.find(a => a.id === id); // External annotations (live in SSE hook) route to the SSE hook, not local state. // Check membership by ID — source alone is insufficient because share-imported @@ -3272,6 +3280,7 @@ const App: React.FC = () => { }; const handleEditAnnotation = (id: string, updates: Partial) => { + if (documentReadOnly) return; const ann = allAnnotations.find(a => a.id === id); if (ann?.source && externalAnnotations.some(e => e.id === id)) { updateExternalAnnotation(id, updates); @@ -3283,19 +3292,22 @@ const App: React.FC = () => { }; const handleIdentityChange = useCallback((oldIdentity: string, newIdentity: string) => { + if (documentReadOnly) return; setAnnotations(prev => prev.map(ann => ann.author === oldIdentity ? { ...ann, author: newIdentity } : ann )); setCodeAnnotations(prev => prev.map(ann => ann.author === oldIdentity ? { ...ann, author: newIdentity } : ann )); - }, []); + }, [documentReadOnly]); const handleAddGlobalAttachment = (image: ImageAttachment) => { + if (documentReadOnly) return; setGlobalAttachments(prev => [...prev, image]); }; const handleRemoveGlobalAttachment = (path: string) => { + if (documentReadOnly) return; setGlobalAttachments(prev => prev.filter(p => p.path !== path)); }; @@ -4595,6 +4607,7 @@ const App: React.FC = () => { diffActive={isPlanDiffActive && !!htmlDiffHtml} onToggleDiff={() => setIsPlanDiffActive((v) => !v)} onAskAI={canUseDocumentAskAI ? handleAskAI : undefined} + readOnly={documentReadOnly} /> ) : isEditingMarkdown ? ( { checkboxOverrides={checkbox.overrides} actionsLabelMode={actionsLabelMode} onAskAI={canUseDocumentAskAI ? handleAskAI : undefined} + readOnly={documentReadOnly} /> )} @@ -4718,6 +4732,7 @@ const App: React.FC = () => { onDiscard: item.id === 'plan' ? () => handleDiscardEdits() : undefined, })) ?? null} onOtherFileAnnotationsClick={handleFlashAnnotatedFiles} + readOnly={documentReadOnly} /> {isPanelOpen && rightSidebarTab === 'ai' && wideModeType === null && !goalSetupMode && canUseAskAI && (