From 6998ce680bfa0ada797cf7bb6d87d6b392006920 Mon Sep 17 00:00:00 2001 From: Ben Newman <158209000+BenNewman100@users.noreply.github.com> Date: Tue, 21 Jul 2026 17:31:48 -0400 Subject: [PATCH 01/15] refactor(annotate): extract per-file version history into a shared helper Move the single-file annotate-history pipeline (slug derivation, saveToHistory, previous-version lookup, degrade-on-error) out of the Bun-specific annotate server and into packages/shared/annotate-history.ts, built on node:fs/node:path/node:crypto only so other runtimes can vendor it unmodified. annotate.ts now calls computeAnnotateHistory() instead of inlining the pipeline; behavior for single-file sessions is unchanged. --- packages/server/annotate.ts | 48 +++------------ packages/shared/annotate-history.ts | 96 +++++++++++++++++++++++++++++ packages/shared/package.json | 1 + 3 files changed, 105 insertions(+), 40 deletions(-) create mode 100644 packages/shared/annotate-history.ts diff --git a/packages/server/annotate.ts b/packages/server/annotate.ts index 69655e652..9465116a0 100644 --- a/packages/server/annotate.ts +++ b/packages/server/annotate.ts @@ -19,7 +19,8 @@ import { handleDoc, handleDocExists, handleFileBrowserFiles, handleObsidianVault import { handleFileBrowserFilesStream } from "./reference-watch"; import { resolveUserPath, warmFileListCache } from "@plannotator/shared/resolve-file"; import { contentHash, deleteDraft } from "./draft"; -import { saveToHistory, getPlanVersion, getVersionCount, listVersions } from "@plannotator/shared/storage"; +import { getPlanVersion, listVersions } from "@plannotator/shared/storage"; +import { computeAnnotateHistory, type AnnotateHistoryResult } from "@plannotator/shared/annotate-history"; import { htmlDiff } from "@plannotator/shared/html-diff"; import { disabledSourceSave, type SourceSaveRequest } from "@plannotator/shared/source-save"; import { getAnnotateReferenceRootPaths } from "@plannotator/shared/annotate-reference-roots-node"; @@ -164,14 +165,7 @@ export async function startAnnotateServer( // when headings change. Diff content is the markdown, or the raw HTML source // when rendering HTML. Only single local files (not URLs/folders/messages). const annotateProjectName = project ?? "_unknown"; - let annotateHistory: - | { - slug: string; - diffCurrent: string; - previousPlan: string | null; - versionInfo: { version: number; totalVersions: number; project: string }; - } - | null = null; + let annotateHistory: AnnotateHistoryResult | null = null; { const historyContent = renderHtml && rawHtml ? rawHtml : markdown; const eligible = @@ -179,38 +173,12 @@ export async function startAnnotateServer( !/^https?:\/\//i.test(filePath) && historyContent.length > 0 && resolveAnnotateHistory(loadConfig()); + // History is an enhancement, never a gate: a read-only/full data dir + // must degrade to v0.22.0's stateless annotate (no version diff), not + // fail the whole session before the UI ever opens. (computeAnnotateHistory + // never throws — it logs and returns null on any storage error.) if (eligible) { - const base = - (filePath.split(/[\\/]/).pop() || "document") - .toLowerCase() - .replace(/[^a-z0-9]+/g, "-") - .replace(/^-+|-+$/g, "") - .slice(0, 60) || "document"; - const slug = `annotate-${base}-${contentHash(resolvePath(filePath)).slice(0, 8)}`; - // History is an enhancement, never a gate: a read-only/full data dir - // must degrade to v0.22.0's stateless annotate (no version diff), not - // fail the whole session before the UI ever opens. - try { - const saved = saveToHistory(annotateProjectName, slug, historyContent); - const previousPlan = - saved.version > 1 - ? getPlanVersion(annotateProjectName, slug, saved.version - 1) - : null; - annotateHistory = { - slug, - diffCurrent: historyContent, - previousPlan, - versionInfo: { - version: saved.version, - totalVersions: getVersionCount(annotateProjectName, slug), - project: annotateProjectName, - }, - }; - } catch (error) { - console.error( - `[plannotator] warning: annotate history unavailable (${error instanceof Error ? error.message : String(error)}); continuing without version diff`, - ); - } + annotateHistory = computeAnnotateHistory(annotateProjectName, resolvePath(filePath), historyContent); } } const draftSource = diff --git a/packages/shared/annotate-history.ts b/packages/shared/annotate-history.ts new file mode 100644 index 000000000..05f0c70f5 --- /dev/null +++ b/packages/shared/annotate-history.ts @@ -0,0 +1,96 @@ +/** + * Annotate per-file version history. + * + * Runtime-agnostic core of the pipeline that powers annotate mode's inline + * round-over-round diff: derive a stable slug for a file, snapshot its + * content into history, and look up the previous version to diff against. + * Used by both the single-file annotate flow (one file per session) and the + * folder annotate flow (many files served lazily via /api/doc). + * + * History is keyed by file PATH, not content, and slug derivation depends + * only on the resolved path — so the same file annotated once as a + * single-file session and once inside a folder session shares one version + * history and the same slug. + * + * Storage is an enhancement, never a gate: `computeAnnotateHistory` never + * throws. Any failure (read-only data dir, full disk, etc.) is logged and + * `null` is returned so the caller can degrade to a plain render with no + * version diff instead of failing the request. + * + * Uses only node:fs / node:path / node:crypto (via ./storage and ./draft) so + * non-Bun runtimes can vendor it unmodified. + */ + +import { saveToHistory, getPlanVersion, getVersionCount } from "./storage"; +import { contentHash } from "./draft"; + +export interface AnnotateVersionInfo { + version: number; + totalVersions: number; + project: string; +} + +export interface AnnotateHistoryResult { + slug: string; + diffCurrent: string; + previousPlan: string | null; + versionInfo: AnnotateVersionInfo; +} + +/** + * Derive the stable history slug for a file from its resolved absolute path. + * + * Takes an already-resolved path — it does no filesystem resolution of its + * own — so callers are responsible for resolving first (e.g. + * `path.resolve(filePath)` at single-file session start, or the resolved + * `filepath` a folder session's /api/doc handler already computed for the + * request). Same input always produces the same slug, which is what lets a + * version saved under one annotate mode surface as the baseline when the + * other mode opens the same path. + */ +export function deriveAnnotateHistorySlug(resolvedFilePath: string): string { + const base = + (resolvedFilePath.split(/[\\/]/).pop() || "document") + .toLowerCase() + .replace(/[^a-z0-9]+/g, "-") + .replace(/^-+|-+$/g, "") + .slice(0, 60) || "document"; + return `annotate-${base}-${contentHash(resolvedFilePath).slice(0, 8)}`; +} + +/** + * Run the save-to-history → previous-version lookup pipeline for one file. + * + * Saves `content` as the next version (storage dedupes identical content + * against the latest stored version), then looks up the previous version (if + * any) to diff against. Never throws — any storage error is logged and + * results in `null`, which callers should treat as "no version diff for this + * request", not a failure of the request itself. + */ +export function computeAnnotateHistory( + project: string, + resolvedFilePath: string, + content: string, +): AnnotateHistoryResult | null { + const slug = deriveAnnotateHistorySlug(resolvedFilePath); + try { + const saved = saveToHistory(project, slug, content); + const previousPlan = + saved.version > 1 ? getPlanVersion(project, slug, saved.version - 1) : null; + return { + slug, + diffCurrent: content, + previousPlan, + versionInfo: { + version: saved.version, + totalVersions: getVersionCount(project, slug), + project, + }, + }; + } catch (error) { + console.error( + `[plannotator] warning: annotate history unavailable (${error instanceof Error ? error.message : String(error)}); continuing without version diff`, + ); + return null; + } +} diff --git a/packages/shared/package.json b/packages/shared/package.json index 06b2d8d34..26b139095 100644 --- a/packages/shared/package.json +++ b/packages/shared/package.json @@ -27,6 +27,7 @@ "./project": "./project.ts", "./storage": "./storage.ts", "./draft": "./draft.ts", + "./annotate-history": "./annotate-history.ts", "./html-diff": "./html-diff.ts", "./integrations-common": "./integrations-common.ts", "./repo": "./repo.ts", From 29f1bcb49bce4fd8648b41708cf28730cb5efb67 Mon Sep 17 00:00:00 2001 From: Ben Newman <158209000+BenNewman100@users.noreply.github.com> Date: Tue, 21 Jul 2026 17:32:52 -0400 Subject: [PATCH 02/15] feat(annotate): extend per-file version history to folder annotate sessions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Eligible folder files served through /api/doc now get snapshotted into the same version history the single-file flow uses, and their doc responses carry the same previousPlan/versionInfo/diffCurrent fields /api/plan already returns for single-file sessions. The pipeline runs lazily on first open and is memoized per resolved absolute path for the life of the server, so reopening a file never re-snapshots it. Eligibility mirrors the single-file source-save gates: a local file under the session's folder root, markdown-branch documents only (.md/.txt, not HTML, not a Turndown-converted doc), under the existing 2MB annotatable-file cap, and gated by the same annotateHistory config toggle. Storage failures degrade to a plain render (never a gate on the request) via the same try/catch computeAnnotateHistory already wraps. /api/plan/version and /api/plan/versions gain an optional path (+ base) query param so folder sessions can ask for a specific file's history; the slug is always derived server-side from the resolved, containment-checked path — never accepted from the client, since it gets joined unsanitized into a filesystem path. Omitting path keeps today's single-session-binding behavior unchanged. --- packages/server/annotate.ts | 72 ++++++++++++++++++-- packages/server/reference-handlers.ts | 94 ++++++++++++++++++++++++--- 2 files changed, 150 insertions(+), 16 deletions(-) diff --git a/packages/server/annotate.ts b/packages/server/annotate.ts index 9465116a0..a6d0f8d39 100644 --- a/packages/server/annotate.ts +++ b/packages/server/annotate.ts @@ -15,12 +15,12 @@ import { isRemoteSession, getServerHostname, startBunServerOnAvailablePort } fro import { getRepoInfo } from "./repo"; import type { Origin } from "@plannotator/shared/agents"; import { handleImage, handleUpload, handleServerReady, handleDraftSave, handleDraftLoad, handleDraftDelete, handleApiNotFound, handleFavicon, handleSaveNotes, readDraftGenerationFromBody, readDraftGenerationFromUrl } from "./shared-handlers"; -import { handleDoc, handleDocExists, handleFileBrowserFiles, handleObsidianVaults, handleObsidianFiles, handleObsidianDoc } from "./reference-handlers"; +import { handleDoc, handleDocExists, handleFileBrowserFiles, handleObsidianVaults, handleObsidianFiles, handleObsidianDoc, resolveAllowedDocPath } from "./reference-handlers"; import { handleFileBrowserFilesStream } from "./reference-watch"; import { resolveUserPath, warmFileListCache } from "@plannotator/shared/resolve-file"; import { contentHash, deleteDraft } from "./draft"; -import { getPlanVersion, listVersions } from "@plannotator/shared/storage"; -import { computeAnnotateHistory, type AnnotateHistoryResult } from "@plannotator/shared/annotate-history"; +import { getPlanVersion, getVersionCount, listVersions } from "@plannotator/shared/storage"; +import { computeAnnotateHistory, deriveAnnotateHistorySlug, type AnnotateHistoryResult } from "@plannotator/shared/annotate-history"; import { htmlDiff } from "@plannotator/shared/html-diff"; import { disabledSourceSave, type SourceSaveRequest } from "@plannotator/shared/source-save"; import { getAnnotateReferenceRootPaths } from "@plannotator/shared/annotate-reference-roots-node"; @@ -165,6 +165,7 @@ export async function startAnnotateServer( // when headings change. Diff content is the markdown, or the raw HTML source // when rendering HTML. Only single local files (not URLs/folders/messages). const annotateProjectName = project ?? "_unknown"; + const annotateHistoryEnabled = resolveAnnotateHistory(loadConfig()); let annotateHistory: AnnotateHistoryResult | null = null; { const historyContent = renderHtml && rawHtml ? rawHtml : markdown; @@ -172,7 +173,7 @@ export async function startAnnotateServer( mode === "annotate" && !/^https?:\/\//i.test(filePath) && historyContent.length > 0 && - resolveAnnotateHistory(loadConfig()); + annotateHistoryEnabled; // History is an enhancement, never a gate: a read-only/full data dir // must degrade to v0.22.0's stateless annotate (no version diff), not // fail the whole session before the UI ever opens. (computeAnnotateHistory @@ -181,6 +182,19 @@ export async function startAnnotateServer( annotateHistory = computeAnnotateHistory(annotateProjectName, resolvePath(filePath), historyContent); } } + + // Folder annotate: the same per-file version history, but run lazily the + // first time a folder file is opened via /api/doc (not eagerly for every + // file in the folder) and memoized per resolved absolute path for the life + // of this server — reopening the same file in this session never re-snapshots. + const folderAnnotateHistoryCache = new Map(); + function computeFolderAnnotateHistory(resolvedFilePath: string, content: string): AnnotateHistoryResult | null { + const cached = folderAnnotateHistoryCache.get(resolvedFilePath); + if (cached !== undefined) return cached; + const result = computeAnnotateHistory(annotateProjectName, resolvedFilePath, content); + folderAnnotateHistoryCache.set(resolvedFilePath, result); + return result; + } const draftSource = mode === "annotate-folder" && folderPath ? `folder:${resolvePath(folderPath)}` @@ -379,16 +393,39 @@ export async function startAnnotateServer( } // API: fetch a specific version of the annotated file (version diff base picker) + // + // Folder sessions pass `?path=` (optionally `&base=`) to identify which + // file's history to read, resolved and containment-checked exactly like + // /api/doc; the slug is always derived server-side from that resolved + // path — a client-supplied slug is never accepted, since getHistoryDir + // joins it into a filesystem path unsanitized. Without `path`, behavior + // is unchanged: the single session's own history is used. if (url.pathname === "/api/plan/version" && req.method === "GET") { - if (!annotateHistory) { - return Response.json({ error: "No version history" }, { status: 404 }); + const pathParam = url.searchParams.get("path"); + let slug: string; + if (pathParam !== null) { + const resolved = resolveAllowedDocPath(pathParam, url.searchParams.get("base"), { + rootPaths: getReferenceRootPaths(), + }); + if (resolved.kind === "denied") { + return Response.json({ error: "Access denied: path is outside project root" }, { status: 403 }); + } + slug = deriveAnnotateHistorySlug(resolved.path); + if (getVersionCount(annotateProjectName, slug) === 0) { + return Response.json({ error: "No version history" }, { status: 404 }); + } + } else { + if (!annotateHistory) { + return Response.json({ error: "No version history" }, { status: 404 }); + } + slug = annotateHistory.slug; } const vParam = url.searchParams.get("v"); const v = vParam ? parseInt(vParam, 10) : NaN; if (isNaN(v) || v < 1) { return new Response("Invalid version number", { status: 400 }); } - const content = getPlanVersion(annotateProjectName, annotateHistory.slug, v); + const content = getPlanVersion(annotateProjectName, slug, v); if (content === null) { return Response.json({ error: "Version not found" }, { status: 404 }); } @@ -396,7 +433,24 @@ export async function startAnnotateServer( } // API: list all stored versions of the annotated file (Version Browser) + // Same `?path=`/`&base=` parameterization as /api/plan/version above. if (url.pathname === "/api/plan/versions" && req.method === "GET") { + const pathParam = url.searchParams.get("path"); + if (pathParam !== null) { + const resolved = resolveAllowedDocPath(pathParam, url.searchParams.get("base"), { + rootPaths: getReferenceRootPaths(), + }); + if (resolved.kind === "denied") { + return Response.json({ error: "Access denied: path is outside project root" }, { status: 403 }); + } + const slug = deriveAnnotateHistorySlug(resolved.path); + const versions = listVersions(annotateProjectName, slug); + return Response.json({ + project: annotateProjectName, + slug: versions.length > 0 ? slug : null, + versions, + }); + } if (!annotateHistory) { return Response.json({ project: annotateProjectName, slug: null, versions: [] }); } @@ -483,6 +537,10 @@ export async function startAnnotateServer( sourceSaveFolderPath: mode === "annotate-folder" ? folderPath : undefined, onSourceDocumentServed: (path) => openedSourceFilePaths.add(path), rootPaths: getReferenceRootPaths(), + annotateHistory: + mode === "annotate-folder" && annotateHistoryEnabled + ? { compute: computeFolderAnnotateHistory } + : undefined, }); } diff --git a/packages/server/reference-handlers.ts b/packages/server/reference-handlers.ts index c500509b9..6d23ba6b7 100644 --- a/packages/server/reference-handlers.ts +++ b/packages/server/reference-handlers.ts @@ -38,16 +38,38 @@ import { readSourceFileSnapshot, resolveExistingSourceSaveFile, } from "@plannotator/shared/source-save-node"; +import type { AnnotateHistoryResult } from "@plannotator/shared/annotate-history"; import { preloadFile } from "@pierre/diffs/ssr"; // --- Route handlers --- +/** + * Extensions eligible for annotate's per-file version history when served + * through /api/doc (folder annotate mode). Deliberately narrower than the + * full annotatable-text set: markdown-branch documents only, matching the + * gate the folder annotate design settled on. + */ +const ANNOTATE_HISTORY_ELIGIBLE_REGEX = /\.(md|txt)$/i; + export interface HandleDocOptions { rewriteHtml?: (html: string, filepath: string) => string; sourceSaveFilePath?: string; sourceSaveFolderPath?: string; onSourceDocumentServed?: (path: string) => void; rootPaths?: string[]; + /** + * When set, /api/doc runs annotate's per-file version-history pipeline for + * eligible markdown-branch documents (local file under an allowed root, + * .md/.txt, not HTML, not a converted doc, under the annotatable size cap + * already enforced above) and merges `previousPlan`/`versionInfo`/ + * `diffCurrent` into the response — the same field names the single-file + * /api/plan payload uses. `compute` is expected to memoize per resolved + * path itself (the annotate server keys its cache by path in its own + * closure); this module only decides *whether* to call it. + */ + annotateHistory?: { + compute: (resolvedFilePath: string, content: string) => AnnotateHistoryResult | null; + }; } interface HandleDocExistsOptions { @@ -84,6 +106,32 @@ function getTrustedBaseDir(base: string | null, roots: string[]): string | null return isWithinAllowedRoots(resolvedBase, roots) ? resolvedBase : null; } +export type ResolveAllowedDocPathResult = + | { kind: "resolved"; path: string } + | { kind: "denied" }; + +/** + * Resolve a client-supplied path the same way /api/doc's base-relative and + * absolute branches do (see `getTrustedBaseDir` / `isWithinAllowedRoots` + * above), for callers that need the canonical contained path without + * reading the file — namely the annotate version endpoints, which derive a + * history slug from the resolved path rather than trusting a client-supplied + * slug (a client slug would be a path-traversal vector: `getHistoryDir` + * joins it into a filesystem path unsanitized). + */ +export function resolveAllowedDocPath( + requestedPath: string, + base: string | null, + options?: { rootPaths?: string[] }, +): ResolveAllowedDocPathResult { + const allowedRoots = getAllowedRootPaths(options); + const resolvedBase = getTrustedBaseDir(base, allowedRoots); + const candidate = resolveUserPath(requestedPath, resolvedBase ?? undefined); + return isWithinAllowedRoots(candidate, allowedRoots) + ? { kind: "resolved", path: candidate } + : { kind: "denied" }; +} + function relativizeToAllowedRoots(path: string, roots: string[]): string { for (const root of roots) { const prefix = `${root}/`; @@ -148,11 +196,18 @@ function resolveMarkdownFileFromAllowedRoots(input: string, roots: string[]): Ro return { kind: "not_found", input }; } +type DocOptionsResult = T & { + sourceSave?: SourceSaveCapability; + previousPlan?: string | null; + versionInfo?: AnnotateHistoryResult["versionInfo"]; + diffCurrent?: string; +}; + function applyDocOptions>( data: T, options: HandleDocOptions = {}, sourceSnapshot?: SourceFileSnapshot, -): T & { sourceSave?: SourceSaveCapability } { +): DocOptionsResult { const next: Record = { ...data }; if ( typeof next.rawHtml === "string" && @@ -161,16 +216,37 @@ function applyDocOptions>( ) { next.rawHtml = options.rewriteHtml(next.rawHtml, next.filepath); } + // Annotate version history (folder mode only — see HandleDocOptions.annotateHistory). + // Independent of the sourceSave branching below: only markdown-branch + // documents (not HTML, not converted) with a .md/.txt extension are + // eligible. The 2MB annotatable-file size cap is already enforced by the + // caller before any of these responses are built, so no separate check is + // needed here. + if ( + options.annotateHistory && + typeof data.filepath === "string" && + data.renderAs === "markdown" && + data.isConverted !== true && + typeof data.markdown === "string" && + ANNOTATE_HISTORY_ELIGIBLE_REGEX.test(data.filepath) + ) { + const history = options.annotateHistory.compute(data.filepath, data.markdown); + if (history) { + next.previousPlan = history.previousPlan; + next.versionInfo = history.versionInfo; + next.diffCurrent = history.diffCurrent; + } + } if (typeof data.filepath !== "string") { - return options.sourceSaveFolderPath || options.sourceSaveFilePath - ? { ...next, sourceSave: disabledSourceSave("not-local-file") } as T & { sourceSave?: SourceSaveCapability } - : next as T & { sourceSave?: SourceSaveCapability }; + return (options.sourceSaveFolderPath || options.sourceSaveFilePath + ? { ...next, sourceSave: disabledSourceSave("not-local-file") } + : next) as DocOptionsResult; } if (data.renderAs === "html") { - return { ...next, sourceSave: disabledSourceSave("html-render") } as T & { sourceSave?: SourceSaveCapability }; + return { ...next, sourceSave: disabledSourceSave("html-render") } as DocOptionsResult; } if (data.isConverted === true) { - return { ...next, sourceSave: disabledSourceSave("converted-source") } as T & { sourceSave?: SourceSaveCapability }; + return { ...next, sourceSave: disabledSourceSave("converted-source") } as DocOptionsResult; } if (options.sourceSaveFilePath) { const sourcePath = resolveExistingSourceSaveFile("single-file", options.sourceSaveFilePath); @@ -179,10 +255,10 @@ function applyDocOptions>( : createSourceSaveCapability("single-file", data.filepath); if (sourcePath && doc.enabled && sourcePath === doc.path) { options.onSourceDocumentServed?.(doc.path); - return { ...next, sourceSave: doc } as T & { sourceSave?: SourceSaveCapability }; + return { ...next, sourceSave: doc } as DocOptionsResult; } } - if (!options.sourceSaveFolderPath) return next as T & { sourceSave?: SourceSaveCapability }; + if (!options.sourceSaveFolderPath) return next as DocOptionsResult; const sourceSave = sourceSnapshot ? createSourceSaveCapabilityFromSnapshot("folder-file", data.filepath, sourceSnapshot, options.sourceSaveFolderPath) : createSourceSaveCapability("folder-file", data.filepath, options.sourceSaveFolderPath); @@ -190,7 +266,7 @@ function applyDocOptions>( return { ...next, sourceSave, - } as T & { sourceSave?: SourceSaveCapability }; + } as DocOptionsResult; } function docJson(data: Record, options?: HandleDocOptions, sourceSnapshot?: SourceFileSnapshot): Response { From 493cbbb3ecb3116c51404f45b004ca8c43c84e67 Mon Sep 17 00:00:00 2001 From: Ben Newman <158209000+BenNewman100@users.noreply.github.com> Date: Tue, 21 Jul 2026 17:33:10 -0400 Subject: [PATCH 03/15] test(annotate): cover folder annotate version history Adds a new describe block exercising the folder-mode history pipeline added in the previous commit: first-open snapshot + same-session memoization, storage-level dedupe, cross-mode slug continuity with the single-file flow, first-ever-open field shape, the config toggle, an ineligible (HTML) file type, degrade-on-unwritable-history-dir, and the path-parameterized version endpoints (including containment rejection and the no-path fallback). --- packages/server/annotate.test.ts | 360 +++++++++++++++++++++++++++++++ 1 file changed, 360 insertions(+) diff --git a/packages/server/annotate.test.ts b/packages/server/annotate.test.ts index 86b3034c4..5dd186246 100644 --- a/packages/server/annotate.test.ts +++ b/packages/server/annotate.test.ts @@ -19,6 +19,8 @@ import { existsSync, mkdirSync, mkdtempSync, readFileSync, realpathSync, symlink import { tmpdir } from "os"; import { join } from "path"; import { startAnnotateServer } from "./annotate"; +import { deriveAnnotateHistorySlug } from "@plannotator/shared/annotate-history"; +import { getPlannotatorDataDir } from "@plannotator/shared/data-dir"; const MINIMAL_HTML = "Plannotator"; @@ -503,3 +505,361 @@ describe("annotate server: source save", () => { } }); }); + +describe("annotate server: folder annotate history", () => { + let savedPort: string | undefined; + let savedRemote: string | undefined; + let savedHistoryFlag: string | undefined; + + beforeEach(() => { + savedPort = process.env.PLANNOTATOR_PORT; + savedRemote = process.env.PLANNOTATOR_REMOTE; + savedHistoryFlag = process.env.PLANNOTATOR_ANNOTATE_HISTORY; + delete process.env.PLANNOTATOR_PORT; + process.env.PLANNOTATOR_REMOTE = "0"; + // Force the toggle on for every test but the one that explicitly flips it + // off — a real ~/.plannotator/config.json on the machine running these + // tests must never change the outcome. + process.env.PLANNOTATOR_ANNOTATE_HISTORY = "1"; + }); + + afterEach(() => { + if (savedPort === undefined) delete process.env.PLANNOTATOR_PORT; + else process.env.PLANNOTATOR_PORT = savedPort; + if (savedRemote === undefined) delete process.env.PLANNOTATOR_REMOTE; + else process.env.PLANNOTATOR_REMOTE = savedRemote; + if (savedHistoryFlag === undefined) delete process.env.PLANNOTATOR_ANNOTATE_HISTORY; + else process.env.PLANNOTATOR_ANNOTATE_HISTORY = savedHistoryFlag; + }); + + // Every test uses its own project namespace (history lives in the real + // ~/.plannotator data dir, same as storage.test.ts) so runs never collide. + function uniqueProject(label: string): string { + return `_annotate_history_test_${label}_${Date.now()}_${Math.random().toString(36).slice(2, 8)}`; + } + + test("first open mints one version; reopening in the same session is memoized (no re-snapshot even if the file changes on disk)", async () => { + const folderPath = mkdtempSync(join(tmpdir(), "plannotator-folder-history-first-open-")); + const docPath = join(folderPath, "note.md"); + writeFileSync(docPath, "V1\n", "utf-8"); + const project = uniqueProject("first-open"); + + const server = await startAnnotateServer({ + markdown: "", + filePath: folderPath, + folderPath, + mode: "annotate-folder", + htmlContent: MINIMAL_HTML, + project, + }); + + try { + const first = await fetch(`${server.url}/api/doc?path=${encodeURIComponent(docPath)}`); + const firstJson = await first.json() as { + markdown?: string; + previousPlan?: string | null; + versionInfo?: { version: number; totalVersions: number; project: string }; + diffCurrent?: string; + }; + expect(firstJson.markdown).toBe("V1\n"); + expect(firstJson.previousPlan).toBeNull(); + expect(firstJson.versionInfo).toEqual({ version: 1, totalVersions: 1, project }); + expect(firstJson.diffCurrent).toBe("V1\n"); + + // Change the file on disk between opens — a re-run of the pipeline + // would mint version 2. Memoization must prevent that. + writeFileSync(docPath, "V2\n", "utf-8"); + + const second = await fetch(`${server.url}/api/doc?path=${encodeURIComponent(docPath)}`); + const secondJson = await second.json() as { + markdown?: string; + previousPlan?: string | null; + versionInfo?: { version: number; totalVersions: number; project: string }; + }; + // The live document content is always read fresh from disk... + expect(secondJson.markdown).toBe("V2\n"); + // ...but the history snapshot/diff fields stay exactly what first-open computed. + expect(secondJson.previousPlan).toBeNull(); + expect(secondJson.versionInfo).toEqual({ version: 1, totalVersions: 1, project }); + + const versions = await fetch(`${server.url}/api/plan/versions?path=${encodeURIComponent(docPath)}`); + const versionsJson = await versions.json() as { versions: unknown[] }; + expect(versionsJson.versions).toHaveLength(1); + } finally { + server.stop(); + } + }); + + test("content matching the latest stored version dedupes (mints nothing) and still serves correct previous-version fields", async () => { + const folderPath = mkdtempSync(join(tmpdir(), "plannotator-folder-history-dedupe-")); + const docPath = join(folderPath, "note.md"); + const project = uniqueProject("dedupe"); + + // Seed history for this exact path via the single-file flow, then make + // the folder file's on-disk content match that stored version exactly. + const seedServer = await startAnnotateServer({ + markdown: "Same\n", + filePath: docPath, + htmlContent: MINIMAL_HTML, + mode: "annotate", + project, + }); + seedServer.stop(); + writeFileSync(docPath, "Same\n", "utf-8"); + + const server = await startAnnotateServer({ + markdown: "", + filePath: folderPath, + folderPath, + mode: "annotate-folder", + htmlContent: MINIMAL_HTML, + project, + }); + + try { + const response = await fetch(`${server.url}/api/doc?path=${encodeURIComponent(docPath)}`); + const json = await response.json() as { + previousPlan?: string | null; + versionInfo?: { version: number; totalVersions: number; project: string }; + }; + // Dedup keeps it at version 1 — the folder open did not mint version 2. + expect(json.versionInfo).toEqual({ version: 1, totalVersions: 1, project }); + expect(json.previousPlan).toBeNull(); + + const versions = await fetch(`${server.url}/api/plan/versions?path=${encodeURIComponent(docPath)}`); + const versionsJson = await versions.json() as { versions: unknown[] }; + expect(versionsJson.versions).toHaveLength(1); + } finally { + server.stop(); + } + }); + + test("cross-mode slug continuity: a version saved via single-file flow is served as the baseline when a folder session opens the same path", async () => { + const folderPath = mkdtempSync(join(tmpdir(), "plannotator-folder-history-cross-mode-")); + const docPath = join(folderPath, "note.md"); + const project = uniqueProject("cross-mode"); + + // Single-file session saves "V1" as version 1 for this exact resolved path. + const seedServer = await startAnnotateServer({ + markdown: "V1\n", + filePath: docPath, + htmlContent: MINIMAL_HTML, + mode: "annotate", + project, + }); + seedServer.stop(); + + // The folder session reads different content off disk, so it mints version 2. + writeFileSync(docPath, "V2\n", "utf-8"); + + const server = await startAnnotateServer({ + markdown: "", + filePath: folderPath, + folderPath, + mode: "annotate-folder", + htmlContent: MINIMAL_HTML, + project, + }); + + try { + const response = await fetch(`${server.url}/api/doc?path=${encodeURIComponent(docPath)}`); + const json = await response.json() as { + previousPlan?: string | null; + versionInfo?: { version: number; totalVersions: number; project: string }; + }; + expect(json.previousPlan).toBe("V1\n"); + expect(json.versionInfo).toEqual({ version: 2, totalVersions: 2, project }); + + const versionOne = await fetch(`${server.url}/api/plan/version?path=${encodeURIComponent(docPath)}&v=1`); + const versionOneJson = await versionOne.json() as { plan?: string }; + expect(versionOneJson.plan).toBe("V1\n"); + } finally { + server.stop(); + } + }); + + test("first-ever open of a never-annotated path carries no previous version but does report version 1 of 1 (parity with single-file)", async () => { + const folderPath = mkdtempSync(join(tmpdir(), "plannotator-folder-history-never-seen-")); + const docPath = join(folderPath, "note.md"); + writeFileSync(docPath, "Fresh\n", "utf-8"); + const project = uniqueProject("never-seen"); + + const server = await startAnnotateServer({ + markdown: "", + filePath: folderPath, + folderPath, + mode: "annotate-folder", + htmlContent: MINIMAL_HTML, + project, + }); + + try { + const response = await fetch(`${server.url}/api/doc?path=${encodeURIComponent(docPath)}`); + const json = await response.json() as { + previousPlan?: string | null; + versionInfo?: { version: number; totalVersions: number; project: string }; + diffCurrent?: string; + }; + expect(json.previousPlan).toBeNull(); + expect(json.versionInfo).toEqual({ version: 1, totalVersions: 1, project }); + expect(json.diffCurrent).toBe("Fresh\n"); + } finally { + server.stop(); + } + }); + + test("config toggle off: no snapshot, no diff fields, doc still serves", async () => { + process.env.PLANNOTATOR_ANNOTATE_HISTORY = "0"; + const folderPath = mkdtempSync(join(tmpdir(), "plannotator-folder-history-toggle-off-")); + const docPath = join(folderPath, "note.md"); + writeFileSync(docPath, "Content\n", "utf-8"); + const project = uniqueProject("toggle-off"); + + const server = await startAnnotateServer({ + markdown: "", + filePath: folderPath, + folderPath, + mode: "annotate-folder", + htmlContent: MINIMAL_HTML, + project, + }); + + try { + const response = await fetch(`${server.url}/api/doc?path=${encodeURIComponent(docPath)}`); + expect(response.status).toBe(200); + const json = await response.json() as Record; + expect(json.markdown).toBe("Content\n"); + expect("previousPlan" in json).toBe(false); + expect("versionInfo" in json).toBe(false); + expect("diffCurrent" in json).toBe(false); + + const versions = await fetch(`${server.url}/api/plan/versions?path=${encodeURIComponent(docPath)}`); + const versionsJson = await versions.json() as { slug: string | null; versions: unknown[] }; + expect(versionsJson).toEqual({ project, slug: null, versions: [] }); + } finally { + server.stop(); + } + }); + + test("ineligible file type (HTML) serves as today with no snapshot", async () => { + const folderPath = mkdtempSync(join(tmpdir(), "plannotator-folder-history-html-")); + const docPath = join(folderPath, "page.html"); + writeFileSync(docPath, "Hi", "utf-8"); + const project = uniqueProject("html"); + + const server = await startAnnotateServer({ + markdown: "", + filePath: folderPath, + folderPath, + mode: "annotate-folder", + htmlContent: MINIMAL_HTML, + project, + }); + + try { + const response = await fetch(`${server.url}/api/doc?path=${encodeURIComponent(docPath)}`); + expect(response.status).toBe(200); + const json = await response.json() as Record; + expect(json.renderAs).toBe("html"); + expect("previousPlan" in json).toBe(false); + expect("versionInfo" in json).toBe(false); + expect("diffCurrent" in json).toBe(false); + + const versions = await fetch(`${server.url}/api/plan/versions?path=${encodeURIComponent(docPath)}`); + const versionsJson = await versions.json() as { slug: string | null; versions: unknown[] }; + expect(versionsJson).toEqual({ project, slug: null, versions: [] }); + } finally { + server.stop(); + } + }); + + test("an unwritable history directory degrades to a plain render, no error propagates", async () => { + const folderPath = mkdtempSync(join(tmpdir(), "plannotator-folder-history-unwritable-")); + const docPath = join(folderPath, "note.md"); + writeFileSync(docPath, "Content\n", "utf-8"); + const project = uniqueProject("unwritable"); + + // Block the exact history directory the pipeline will try to mkdir by + // pre-creating a plain FILE at that path — mkdirSync(recursive) throws + // when a target segment exists and is not a directory, on every platform. + const slug = deriveAnnotateHistorySlug(docPath); + const historyProjectDir = join(getPlannotatorDataDir(), "history", project); + mkdirSync(historyProjectDir, { recursive: true }); + writeFileSync(join(historyProjectDir, slug), "not a directory", "utf-8"); + + const server = await startAnnotateServer({ + markdown: "", + filePath: folderPath, + folderPath, + mode: "annotate-folder", + htmlContent: MINIMAL_HTML, + project, + }); + + try { + const response = await fetch(`${server.url}/api/doc?path=${encodeURIComponent(docPath)}`); + expect(response.status).toBe(200); + const json = await response.json() as Record; + expect(json.markdown).toBe("Content\n"); + expect("previousPlan" in json).toBe(false); + expect("versionInfo" in json).toBe(false); + expect("diffCurrent" in json).toBe(false); + } finally { + server.stop(); + } + }); + + test("version endpoints: path param serves that file's versions; without path, single-session binding is unchanged", async () => { + const folderPath = mkdtempSync(join(tmpdir(), "plannotator-folder-history-endpoints-")); + const docPath = join(folderPath, "note.md"); + writeFileSync(docPath, "V1\n", "utf-8"); + const project = uniqueProject("endpoints"); + + const server = await startAnnotateServer({ + markdown: "", + filePath: folderPath, + folderPath, + mode: "annotate-folder", + htmlContent: MINIMAL_HTML, + project, + }); + + try { + // No history yet for this path: version endpoints report empty, not an error. + const versionsBefore = await fetch(`${server.url}/api/plan/versions?path=${encodeURIComponent(docPath)}`); + expect(await versionsBefore.json()).toEqual({ project, slug: null, versions: [] }); + const versionBefore = await fetch(`${server.url}/api/plan/version?path=${encodeURIComponent(docPath)}&v=1`); + expect(versionBefore.status).toBe(404); + expect(await versionBefore.json()).toEqual({ error: "No version history" }); + + // Open the file so history is initialized this session. + await fetch(`${server.url}/api/doc?path=${encodeURIComponent(docPath)}`); + + const versionsAfter = await fetch(`${server.url}/api/plan/versions?path=${encodeURIComponent(docPath)}`); + const versionsAfterJson = await versionsAfter.json() as { slug: string | null; versions: { version: number }[] }; + expect(versionsAfterJson.slug).not.toBeNull(); + expect(versionsAfterJson.versions).toHaveLength(1); + + const versionAfter = await fetch(`${server.url}/api/plan/version?path=${encodeURIComponent(docPath)}&v=1`); + expect(versionAfter.status).toBe(200); + expect(await versionAfter.json()).toEqual({ plan: "V1\n", version: 1 }); + + // A path outside the folder root is rejected the same way /api/doc rejects it. + const outsidePath = join(realpathSync(tmpdir()), "outside.md"); + const deniedVersions = await fetch(`${server.url}/api/plan/versions?path=${encodeURIComponent(outsidePath)}`); + expect(deniedVersions.status).toBe(403); + const deniedVersion = await fetch(`${server.url}/api/plan/version?path=${encodeURIComponent(outsidePath)}&v=1`); + expect(deniedVersion.status).toBe(403); + + // Without a path param at all, behavior is exactly today's: this + // session has no single-file annotateHistory binding (it's a folder + // session), so both endpoints report "no history" as before. + const noPathVersions = await fetch(`${server.url}/api/plan/versions`); + expect(await noPathVersions.json()).toEqual({ project, slug: null, versions: [] }); + const noPathVersion = await fetch(`${server.url}/api/plan/version?v=1`); + expect(noPathVersion.status).toBe(404); + } finally { + server.stop(); + } + }); +}); From bbf317864653c24c3417f47ff5a6467452b795ed Mon Sep 17 00:00:00 2001 From: Ben Newman <158209000+BenNewman100@users.noreply.github.com> Date: Tue, 21 Jul 2026 18:28:59 -0400 Subject: [PATCH 04/15] feat(ui): add a docKey seam to usePlanDiff for per-document resets usePlanDiff's diff-base state (diffBasePlan, diffBaseVersion, versions, ...) was seeded once from its constructor args and only ever synced later via a "still falsy" guard - fine for a single root document, but switching to a different document (a different previousPlan/versionInfo) would silently keep the previous document's diff base around instead of adopting the new one's. Add an optional docKey param identifying which document the current previousPlan/versionInfo belong to. When it changes between renders, reset diffBasePlan/diffBaseVersion/versions (and in-flight loading/selecting flags) to the newly-provided values. Omitting docKey (or keeping it stable) preserves exactly today's one-time-hydration behavior, so the root document's call site is unaffected until it opts in. No caller passes docKey yet - this is purely additive. --- packages/ui/hooks/usePlanDiff.test.tsx | 276 +++++++++++++++++++++++++ packages/ui/hooks/usePlanDiff.ts | 41 +++- 2 files changed, 315 insertions(+), 2 deletions(-) create mode 100644 packages/ui/hooks/usePlanDiff.test.tsx diff --git a/packages/ui/hooks/usePlanDiff.test.tsx b/packages/ui/hooks/usePlanDiff.test.tsx new file mode 100644 index 000000000..0b4856143 --- /dev/null +++ b/packages/ui/hooks/usePlanDiff.test.tsx @@ -0,0 +1,276 @@ +import { afterEach, describe, expect, test } from 'bun:test'; +import React from 'react'; +import { createRoot, type Root } from 'react-dom/client'; +import { act } from 'react'; +import { usePlanDiff, type PlanDiffFetchers, type UsePlanDiffReturn, type VersionInfo } from './usePlanDiff'; + +// usePlanDiff had no test coverage before this feature. These cover the +// per-document seam added for annotate folder sessions: the `docKey` reset +// (so switching documents doesn't inherit the previous document's diff +// base), that a stable docKey survives a reconcile-style text update without +// losing its baseline, and that per-document `fetchers` are actually used +// instead of the bare-endpoint defaults. + +const hasDom = typeof document !== 'undefined'; + +interface HarnessProps { + currentPlan: string; + initialPreviousPlan: string | null; + versionInfo: VersionInfo | null; + fetchers?: PlanDiffFetchers; + docKey?: string | null; +} + +let root: Root | null = null; +let host: HTMLElement | null = null; + +// A stable component identity is required across mount + update renders — +// re-declaring the component function per call (as react-dom would treat as +// a type change) would remount the hook and lose its state instead of +// re-rendering it, defeating the point of these tests. +function HarnessHolder(props: HarnessProps & { onLatest?: (v: UsePlanDiffReturn) => void }) { + const v = usePlanDiff(props.currentPlan, props.initialPreviousPlan, props.versionInfo, props.fetchers, props.docKey); + props.onLatest?.(v); + return null; +} + +async function mountHolder(props: HarnessProps): Promise<{ current: () => UsePlanDiffReturn; update: (p: HarnessProps) => Promise }> { + host = document.createElement('div'); + document.body.appendChild(host); + root = createRoot(host); + let latest: UsePlanDiffReturn | null = null; + + const render = async (p: HarnessProps) => { + await act(async () => { + root!.render( { latest = v; }} />); + }); + }; + + await render(props); + + return { + current: () => { + if (!latest) throw new Error('hook was not mounted'); + return latest; + }, + update: render, + }; +} + +afterEach(async () => { + if (root) { + await act(async () => { + root!.unmount(); + }); + } + root = null; + host?.remove(); + host = null; +}); + +describe('usePlanDiff — per-document docKey reset', () => { + test.skipIf(!hasDom)('switching docKey resets diff-base state to the new document\'s own baseline', async () => { + const versionInfoA: VersionInfo = { version: 2, totalVersions: 2, project: 'demo' }; + const session = await mountHolder({ + currentPlan: 'doc A v2 text', + initialPreviousPlan: 'doc A v1 text', + versionInfo: versionInfoA, + docKey: 'docA', + }); + + expect(session.current().diffBasePlan).toBe('doc A v1 text'); + expect(session.current().diffBaseVersion).toBe(1); + expect(session.current().hasPreviousVersion).toBe(true); + + // Switch to a different document with no history at all. + await session.update({ + currentPlan: 'doc B text', + initialPreviousPlan: null, + versionInfo: null, + docKey: 'docB', + }); + + expect(session.current().diffBasePlan).toBeNull(); + expect(session.current().diffBaseVersion).toBeNull(); + expect(session.current().hasPreviousVersion).toBe(false); + expect(session.current().versions).toEqual([]); + }); + + test.skipIf(!hasDom)('switching docKey picks up the new document\'s own previousPlan, not the old one', async () => { + const versionInfoA: VersionInfo = { version: 2, totalVersions: 2, project: 'demo' }; + const versionInfoB: VersionInfo = { version: 3, totalVersions: 3, project: 'demo' }; + const session = await mountHolder({ + currentPlan: 'doc A text', + initialPreviousPlan: 'doc A previous', + versionInfo: versionInfoA, + docKey: 'docA', + }); + expect(session.current().diffBasePlan).toBe('doc A previous'); + + await session.update({ + currentPlan: 'doc B text', + initialPreviousPlan: 'doc B previous', + versionInfo: versionInfoB, + docKey: 'docB', + }); + + expect(session.current().diffBasePlan).toBe('doc B previous'); + expect(session.current().diffBaseVersion).toBe(2); + }); + + test.skipIf(!hasDom)('an undefined/stable docKey (the root document) keeps today\'s one-time-hydration behavior', async () => { + // The root document (single-file annotate, plan, review) never passes a + // docKey — this reproduces its call shape exactly. + const session = await mountHolder({ + currentPlan: 'root plan text', + initialPreviousPlan: null, + versionInfo: null, + }); + expect(session.current().diffBasePlan).toBeNull(); + + // /api/plan resolves asynchronously after mount and previousPlan/versionInfo + // arrive — the one-time sync effect should hydrate diffBasePlan. + const versionInfo: VersionInfo = { version: 2, totalVersions: 2, project: 'demo' }; + await session.update({ + currentPlan: 'root plan text', + initialPreviousPlan: 'root previous plan', + versionInfo, + }); + expect(session.current().diffBasePlan).toBe('root previous plan'); + + // A later change to initialPreviousPlan must NOT overwrite an + // already-hydrated diffBasePlan — this is the pre-existing "only sync + // once" behavior the docKey seam must not disturb for the root document. + await session.update({ + currentPlan: 'root plan text', + initialPreviousPlan: 'a different previous plan', + versionInfo, + }); + expect(session.current().diffBasePlan).toBe('root previous plan'); + }); +}); + +describe('usePlanDiff — baseline survives a live-reload-style text update', () => { + test.skipIf(!hasDom)('a currentPlan change with a stable docKey recomputes the diff without losing diffBasePlan', async () => { + const versionInfoA: VersionInfo = { version: 2, totalVersions: 2, project: 'demo' }; + const session = await mountHolder({ + currentPlan: '# Doc\n\noriginal text\n', + initialPreviousPlan: '# Doc\n\nprevious text\n', + versionInfo: versionInfoA, + docKey: 'docA', + }); + + expect(session.current().diffBasePlan).toBe('# Doc\n\nprevious text\n'); + const firstStats = session.current().diffStats; + expect(firstStats).not.toBeNull(); + + // Simulate a reconcile-triggered disk update: only currentPlan changes, + // docKey (the active document's identity) stays the same. + await session.update({ + currentPlan: '# Doc\n\nreloaded text from disk\n', + initialPreviousPlan: '# Doc\n\nprevious text\n', + versionInfo: versionInfoA, + docKey: 'docA', + }); + + // Baseline is unchanged — the diff is still against the ORIGINAL previous + // version, not reset just because the current text moved. + expect(session.current().diffBasePlan).toBe('# Doc\n\nprevious text\n'); + // But the diff itself recomputed against the new current text. + expect(session.current().diffBlocks).not.toBeNull(); + const blocks = session.current().diffBlocks!; + const hasReloadedText = blocks.some((b) => + (b.content ?? '').includes('reloaded text from disk') + ); + expect(hasReloadedText).toBe(true); + }); +}); + +describe('usePlanDiff — per-document fetchers', () => { + test.skipIf(!hasDom)('selectBaseVersion uses the provided fetchVersion instead of the bare-endpoint default', async () => { + let calledWith: number | null = null; + const fetchers: PlanDiffFetchers = { + fetchVersion: async (version) => { + calledWith = version; + return { plan: 'fetched via custom fetcher', version }; + }, + }; + const originalFetch = globalThis.fetch; + Object.defineProperty(globalThis, 'fetch', { + configurable: true, + writable: true, + value: async () => { + throw new Error('default fetch should not be called when a per-document fetcher is supplied'); + }, + }); + + try { + const session = await mountHolder({ + currentPlan: 'current', + initialPreviousPlan: 'previous', + versionInfo: { version: 2, totalVersions: 2, project: 'demo' }, + docKey: 'docA', + fetchers, + }); + + await act(async () => { + await session.current().selectBaseVersion(1); + }); + + expect(calledWith).toBe(1); + expect(session.current().diffBasePlan).toBe('fetched via custom fetcher'); + } finally { + Object.defineProperty(globalThis, 'fetch', { + configurable: true, + writable: true, + value: originalFetch, + }); + } + }); + + test.skipIf(!hasDom)('fetchVersions uses the provided fetcher instead of the bare-endpoint default', async () => { + const fetchers: PlanDiffFetchers = { + fetchVersions: async () => ({ + project: 'demo', + slug: 'annotate-docA', + versions: [ + { version: 1, timestamp: '2026-01-01T00:00:00.000Z' }, + { version: 2, timestamp: '2026-01-02T00:00:00.000Z' }, + ], + }), + }; + const originalFetch = globalThis.fetch; + Object.defineProperty(globalThis, 'fetch', { + configurable: true, + writable: true, + value: async () => { + throw new Error('default fetch should not be called when a per-document fetcher is supplied'); + }, + }); + + try { + const session = await mountHolder({ + currentPlan: 'current', + initialPreviousPlan: 'previous', + versionInfo: { version: 2, totalVersions: 2, project: 'demo' }, + docKey: 'docA', + fetchers, + }); + + await act(async () => { + await session.current().fetchVersions(); + }); + + expect(session.current().versions).toEqual([ + { version: 1, timestamp: '2026-01-01T00:00:00.000Z' }, + { version: 2, timestamp: '2026-01-02T00:00:00.000Z' }, + ]); + } finally { + Object.defineProperty(globalThis, 'fetch', { + configurable: true, + writable: true, + value: originalFetch, + }); + } + }); +}); diff --git a/packages/ui/hooks/usePlanDiff.ts b/packages/ui/hooks/usePlanDiff.ts index a569b6874..883650008 100644 --- a/packages/ui/hooks/usePlanDiff.ts +++ b/packages/ui/hooks/usePlanDiff.ts @@ -5,7 +5,7 @@ * Consumes the version history API endpoints. */ -import { useState, useMemo, useCallback, useEffect } from "react"; +import { useState, useMemo, useCallback, useEffect, useRef } from "react"; import { computePlanDiff, type PlanDiffBlock, @@ -89,7 +89,19 @@ export function usePlanDiff( currentPlan: string, initialPreviousPlan: string | null, versionInfo: VersionInfo | null, - fetchers?: PlanDiffFetchers + fetchers?: PlanDiffFetchers, + /** + * Identity of the document `initialPreviousPlan`/`versionInfo` belong to + * (e.g. a folder/linked document's filepath). When this changes between + * renders, the diff-base state below resets to the newly-provided + * `initialPreviousPlan`/`versionInfo` instead of keeping whatever the + * previously active document had selected — otherwise switching documents + * would silently keep diffing the new document's text against the old + * document's base plan. Omit (or keep it referentially stable, as the root + * document does for the life of a session) to preserve exactly today's + * one-time-hydration behavior via the two sync effects below. + */ + docKey?: string | null ): UsePlanDiffReturn { const fetchVersionImpl = fetchers?.fetchVersion ?? defaultFetchVersion; const fetchVersionsImpl = fetchers?.fetchVersions ?? defaultFetchVersions; @@ -118,6 +130,31 @@ export function usePlanDiff( } }, [versionInfo]); + // Reset diff-base state whenever the active document identity changes + // (e.g. folder/linked-doc navigation). docKey is undefined/stable for a + // session with no per-doc identity (the root document), so this never + // fires there and the sync effects above keep owning its one-time + // hydration — byte-identical to before this seam existed. + const prevDocKeyRef = useRef(docKey); + useEffect(() => { + if (prevDocKeyRef.current === docKey) return; + prevDocKeyRef.current = docKey; + setDiffBasePlan(initialPreviousPlan); + setDiffBaseVersion( + versionInfo && versionInfo.version > 1 ? versionInfo.version - 1 : null + ); + setVersions([]); + setIsLoadingVersions(false); + setIsSelectingVersion(false); + setFetchingVersion(null); + // Only the identity change (docKey) should trigger this reset — the sync + // effects above already handle initialPreviousPlan/versionInfo arriving + // late for a stable identity, and re-running this on every value change + // would fight version selection (selectBaseVersion intentionally leaves + // docKey unchanged while it swaps diffBasePlan/diffBaseVersion). + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [docKey]); + const hasPreviousVersion = versionInfo !== null && versionInfo.totalVersions > 1 && diffBasePlan !== null; From cb40ff272d695a3cdf7a15aa2c110bc369cebef1 Mon Sep 17 00:00:00 2001 From: Ben Newman <158209000+BenNewman100@users.noreply.github.com> Date: Tue, 21 Jul 2026 18:29:12 -0400 Subject: [PATCH 05/15] feat(ui): carry a per-document version-diff baseline through useLinkedDoc /api/doc now returns previousPlan/versionInfo/diffCurrent for eligible folder files (same shape /api/plan already returns for single-file sessions). Extend LinkedDocLoadData with those fields and carry them through the same activate/cache/back lifecycle annotations and markdown already use, so a document's diff baseline: - is captured once when the document is first opened - persists in the per-filepath cache across back()/re-open, instead of being lost or needing a re-fetch - resolves cache-first via the new resolveDiffBaseline helper, gated on whether a baseline was ever captured (versionInfo presence) rather than truthiness of previousPlan - a document at its first-ever version legitimately caches previousPlan: null, which is a resolved fact, not a cache miss The hook exposes the active document's baseline as diffPreviousPlan/ diffVersionInfo, both null when no document is active or the active one has no eligible history (every non-folder linked doc, since /api/doc never populates these fields for those). Not yet consumed by App.tsx - purely additive. --- .../hooks/useLinkedDoc.diffBaseline.test.ts | 59 +++++ packages/ui/hooks/useLinkedDoc.test.tsx | 207 ++++++++++++++++++ packages/ui/hooks/useLinkedDoc.ts | 70 +++++- 3 files changed, 335 insertions(+), 1 deletion(-) create mode 100644 packages/ui/hooks/useLinkedDoc.diffBaseline.test.ts create mode 100644 packages/ui/hooks/useLinkedDoc.test.tsx diff --git a/packages/ui/hooks/useLinkedDoc.diffBaseline.test.ts b/packages/ui/hooks/useLinkedDoc.diffBaseline.test.ts new file mode 100644 index 000000000..d46d281b1 --- /dev/null +++ b/packages/ui/hooks/useLinkedDoc.diffBaseline.test.ts @@ -0,0 +1,59 @@ +import { describe, expect, test } from "bun:test"; +import { resolveDiffBaseline } from "./useLinkedDoc"; + +// Pure-logic coverage for the cache-first selection resolveDiffBaseline +// performs inside activateDocument: a document's version-diff baseline +// (previousPlan/versionInfo) should be set once per document per session +// and survive re-opening it, instead of being lost or re-requested on +// every navigation. See useLinkedDoc.ts's CachedDocState/LinkedDocLoadData +// doc comments for the field shapes this mirrors. + +const VERSION_INFO_A = { version: 2, totalVersions: 2, project: "demo" }; +const VERSION_INFO_B = { version: 3, totalVersions: 3, project: "demo" }; + +describe("resolveDiffBaseline", () => { + test("no cache entry — falls through to the freshly-fetched data", () => { + const result = resolveDiffBaseline(undefined, { + previousPlan: "old text", + versionInfo: VERSION_INFO_A, + }); + expect(result).toEqual({ previousPlan: "old text", versionInfo: VERSION_INFO_A }); + }); + + test("cached baseline wins over a fresh fetch — re-opening a document doesn't refetch its baseline", () => { + const result = resolveDiffBaseline( + { previousPlan: "cached previous", versionInfo: VERSION_INFO_A }, + { previousPlan: "different fetched text", versionInfo: VERSION_INFO_B }, + ); + expect(result).toEqual({ previousPlan: "cached previous", versionInfo: VERSION_INFO_A }); + }); + + test("cache entry exists but has no diff fields (e.g. opened via a non-diff-aware path) — falls through to fetched data", () => { + const result = resolveDiffBaseline( + { previousPlan: undefined, versionInfo: undefined }, + { previousPlan: "fetched text", versionInfo: VERSION_INFO_B }, + ); + expect(result).toEqual({ previousPlan: "fetched text", versionInfo: VERSION_INFO_B }); + }); + + test("neither cache nor data has diff fields — a document with no eligible history stays null/undefined", () => { + const result = resolveDiffBaseline(undefined, { previousPlan: undefined, versionInfo: undefined }); + expect(result.previousPlan).toBeUndefined(); + expect(result.versionInfo).toBeUndefined(); + }); + + test("a first-ever-open document (version 1, no history yet) — cached null previousPlan is preserved, not treated as absent", () => { + // The server can legitimately report previousPlan: null (first version, + // nothing to diff against yet). Since the whole point of caching is that + // a refetch of the same file is idempotent (the server memoizes per + // resolved path for the life of the process), re-deriving from a second + // fetch would return the identical null anyway — but the cached value + // must still be what's used, not silently dropped for being falsy. + const result = resolveDiffBaseline( + { previousPlan: null, versionInfo: { version: 1, totalVersions: 1, project: "demo" } }, + { previousPlan: "should not be used", versionInfo: VERSION_INFO_B }, + ); + expect(result.previousPlan).toBeNull(); + expect(result.versionInfo).toEqual({ version: 1, totalVersions: 1, project: "demo" }); + }); +}); diff --git a/packages/ui/hooks/useLinkedDoc.test.tsx b/packages/ui/hooks/useLinkedDoc.test.tsx new file mode 100644 index 000000000..4b274b167 --- /dev/null +++ b/packages/ui/hooks/useLinkedDoc.test.tsx @@ -0,0 +1,207 @@ +import { afterEach, describe, expect, test } from 'bun:test'; +import React, { useRef, useState } from 'react'; +import { createRoot, type Root } from 'react-dom/client'; +import { act } from 'react'; +import { useLinkedDoc, type UseLinkedDocReturn } from './useLinkedDoc'; +import type { ViewerHandle } from '../components/Viewer'; +import type { Annotation, ImageAttachment } from '../types'; + +// Integration coverage for the diff-baseline seam useLinkedDoc gained for +// annotate folder sessions: a document's own previousPlan/versionInfo +// (captured from its /api/doc response) rides the same +// activate/cache/back lifecycle as its markdown and annotations, and +// survives re-opening a document the way those already did. The pure +// cache-selection logic itself is covered directly in +// useLinkedDoc.diffBaseline.test.ts — this file exercises it wired through +// the real stateful hook (open/back/re-open), which is where a wiring +// mistake (e.g. forgetting one of the cache-write call sites) would show up. + +const hasDom = typeof document !== 'undefined'; + +const noopViewerHandle: ViewerHandle = { + removeHighlight: () => {}, + clearAllHighlights: () => {}, + applySharedAnnotations: () => {}, +}; + +function LinkedDocHarness(props: { onLatest: (v: UseLinkedDocReturn) => void }) { + const [markdown, setMarkdown] = useState('root markdown'); + const [annotations, setAnnotations] = useState([]); + const [selectedAnnotationId, setSelectedAnnotationId] = useState(null); + const [globalAttachments, setGlobalAttachments] = useState([]); + const [renderAs, setRenderAs] = useState<'markdown' | 'html'>('markdown'); + const [rawHtml, setRawHtml] = useState(''); + const [shareHtml, setShareHtml] = useState(''); + const viewerRef = useRef(noopViewerHandle); + + const hook = useLinkedDoc({ + markdown, + annotations, + selectedAnnotationId, + globalAttachments, + setMarkdown, + setAnnotations, + setSelectedAnnotationId, + setGlobalAttachments, + renderAs, + rawHtml, + shareHtml, + setRenderAs, + setRawHtml, + setShareHtml, + viewerRef, + sidebar: { open: () => {} }, + }); + + props.onLatest(hook); + return null; +} + +let root: Root | null = null; +let host: HTMLElement | null = null; + +async function mountHarness(): Promise<() => UseLinkedDocReturn> { + host = document.createElement('div'); + document.body.appendChild(host); + root = createRoot(host); + + let latest: UseLinkedDocReturn | null = null; + await act(async () => { + root!.render( { latest = v; }} />); + }); + + return () => { + if (!latest) throw new Error('hook was not mounted'); + return latest; + }; +} + +afterEach(async () => { + if (root) { + await act(async () => { + root!.unmount(); + }); + } + root = null; + host?.remove(); + host = null; +}); + +const VERSION_INFO_A = { version: 2, totalVersions: 2, project: 'demo' }; + +describe('useLinkedDoc — per-document diff baseline', () => { + test.skipIf(!hasDom)('opening a folder document with history fields flows diffPreviousPlan/diffVersionInfo', async () => { + const current = await mountHarness(); + + await act(async () => { + current().openLoaded({ + filepath: '/root/docs/a.md', + markdown: 'a v2', + renderAs: 'markdown', + previousPlan: 'a v1', + versionInfo: VERSION_INFO_A, + }, undefined, { notifyDocumentLoaded: false }); + }); + + expect(current().isActive).toBe(true); + expect(current().diffPreviousPlan).toBe('a v1'); + expect(current().diffVersionInfo).toEqual(VERSION_INFO_A); + }); + + test.skipIf(!hasDom)('opening a document with no history fields yields no diff baseline', async () => { + const current = await mountHarness(); + + await act(async () => { + current().openLoaded({ + filepath: '/root/docs/b.md', + markdown: 'b', + renderAs: 'markdown', + }, undefined, { notifyDocumentLoaded: false }); + }); + + expect(current().isActive).toBe(true); + expect(current().diffPreviousPlan).toBeNull(); + expect(current().diffVersionInfo).toBeNull(); + }); + + test.skipIf(!hasDom)('no active document (root) reports no diff baseline from the hook itself', async () => { + const current = await mountHarness(); + expect(current().isActive).toBe(false); + expect(current().diffPreviousPlan).toBeNull(); + expect(current().diffVersionInfo).toBeNull(); + }); + + test.skipIf(!hasDom)('back() then re-opening the same document without history fields reuses the cached baseline', async () => { + const current = await mountHarness(); + + await act(async () => { + current().openLoaded({ + filepath: '/root/docs/a.md', + markdown: 'a v2', + renderAs: 'markdown', + previousPlan: 'a v1', + versionInfo: VERSION_INFO_A, + }, undefined, { notifyDocumentLoaded: false }); + }); + expect(current().diffPreviousPlan).toBe('a v1'); + + await act(async () => { + current().back(); + }); + expect(current().isActive).toBe(false); + + // Re-open the SAME file without diff fields this time (simulating a + // caller like the missing-on-disk openLoaded path, which never carries + // /api/doc's history fields) — the cache from the first visit must + // still supply them. + await act(async () => { + current().openLoaded({ + filepath: '/root/docs/a.md', + markdown: 'a v2 (from cache)', + renderAs: 'markdown', + }, undefined, { notifyDocumentLoaded: false }); + }); + + expect(current().diffPreviousPlan).toBe('a v1'); + expect(current().diffVersionInfo).toEqual(VERSION_INFO_A); + }); + + test.skipIf(!hasDom)('switching directly from a document with history to one without (no back() in between) does not leak the old baseline', async () => { + const current = await mountHarness(); + + await act(async () => { + current().openLoaded({ + filepath: '/root/docs/a.md', + markdown: 'a v2', + renderAs: 'markdown', + previousPlan: 'a v1', + versionInfo: VERSION_INFO_A, + }, undefined, { notifyDocumentLoaded: false }); + }); + expect(current().diffPreviousPlan).toBe('a v1'); + + // Open a different document directly (activateDocument's "already + // viewing a linked doc" branch, not back()). + await act(async () => { + current().openLoaded({ + filepath: '/root/docs/c.md', + markdown: 'c', + renderAs: 'markdown', + }, undefined, { notifyDocumentLoaded: false }); + }); + + expect(current().diffPreviousPlan).toBeNull(); + expect(current().diffVersionInfo).toBeNull(); + + // And back to the first document — its baseline is still cached. + await act(async () => { + current().openLoaded({ + filepath: '/root/docs/a.md', + markdown: 'a v2', + renderAs: 'markdown', + }, undefined, { notifyDocumentLoaded: false }); + }); + expect(current().diffPreviousPlan).toBe('a v1'); + expect(current().diffVersionInfo).toEqual(VERSION_INFO_A); + }); +}); diff --git a/packages/ui/hooks/useLinkedDoc.ts b/packages/ui/hooks/useLinkedDoc.ts index 38e739f55..ee47b9a8e 100644 --- a/packages/ui/hooks/useLinkedDoc.ts +++ b/packages/ui/hooks/useLinkedDoc.ts @@ -11,6 +11,7 @@ import type { Annotation, ImageAttachment } from "../types"; import type { ViewerHandle } from "../components/Viewer"; import type { SidebarTab } from "./useSidebar"; import type { SourceSaveCapability } from "@plannotator/core/source-save"; +import type { VersionInfo } from "./usePlanDiff"; export interface LinkedDocLoadData { markdown?: string; @@ -20,6 +21,44 @@ export interface LinkedDocLoadData { rawHtml?: string; shareHtml?: string; sourceSave?: SourceSaveCapability; + /** + * Per-file version-diff baseline (annotate folder sessions only) — the same + * field names/shapes /api/plan already returns for single-file sessions. + * /api/doc only populates these for eligible folder files (local .md/.txt, + * markdown-branch, not converted/HTML); every other document — including + * plain linked docs outside folder mode — omits them, which is what keeps + * the diff badge/version browser from appearing for those. + */ + previousPlan?: string | null; + versionInfo?: VersionInfo | null; + /** Present alongside previousPlan/versionInfo; not currently consumed by + * the client (mirrors the unused /api/plan field for shape parity). */ + diffCurrent?: string; +} + +/** + * Resolve the version-diff baseline for a document being activated: prefer + * an already-cached baseline (set the first time this document was opened) + * over the freshly-fetched `data`, so re-opening a document during a session + * never loses — or needs to re-request — its baseline. Pure so it's testable + * without mounting the hook; used by activateDocument below. + * + * Gated on whether a baseline was EVER captured for this document (versionInfo + * presence), not on truthiness of the cached previousPlan value — a document + * at its first-ever version legitimately caches `previousPlan: null` ("no + * earlier version to diff against yet"), which is a resolved fact, not a + * cache miss. Falling through to `data` on a plain `??` would silently + * discard that fact whenever `data` lacks the fields too (e.g. the + * missing-on-disk openLoaded path, which never fetches /api/doc). + */ +export function resolveDiffBaseline( + cached: Pick | undefined, + data: Pick, +): { previousPlan: string | null | undefined; versionInfo: VersionInfo | null | undefined } { + if (cached && cached.versionInfo !== undefined) { + return { previousPlan: cached.previousPlan, versionInfo: cached.versionInfo }; + } + return { previousPlan: data.previousPlan, versionInfo: data.versionInfo }; } export interface UseLinkedDocOptions { @@ -73,6 +112,12 @@ export interface CachedDocState { globalAttachments: ImageAttachment[]; markdown?: string; isConverted?: boolean; + /** Version-diff baseline captured the first time this document was + * activated — see LinkedDocLoadData. Persisted here so re-opening the + * same document later in the session reuses it instead of losing it (or + * needing to re-fetch it) once the fresh /api/doc data is gone. */ + previousPlan?: string | null; + versionInfo?: VersionInfo | null; } export interface LinkedDocSessionState { @@ -109,6 +154,12 @@ export interface UseLinkedDocReturn { restoreSession: (state: LinkedDocSessionState) => void; /** Reactive count of annotations on non-active documents (updates on open() and back()) */ docAnnotationCount: number; + /** Active document's version-diff baseline (folder annotate only). Null + * when no document is active, or the active document has no eligible + * history — including every non-folder linked doc, since /api/doc never + * populates these fields for those. */ + diffPreviousPlan: string | null; + diffVersionInfo: VersionInfo | null; } const HIGHLIGHT_REAPPLY_DELAY = 100; @@ -139,7 +190,13 @@ export function useLinkedDoc(options: UseLinkedDocOptions): UseLinkedDocReturn { onAfterBack, } = options; - const [linkedDoc, setLinkedDoc] = useState<{ filepath: string; isConverted?: boolean; markdown?: string } | null>(null); + const [linkedDoc, setLinkedDoc] = useState<{ + filepath: string; + isConverted?: boolean; + markdown?: string; + previousPlan?: string | null; + versionInfo?: VersionInfo | null; + } | null>(null); const [error, setError] = useState(null); const [isLoading, setIsLoading] = useState(false); const [docAnnotationCount, setDocAnnotationCount] = useState(0); @@ -169,6 +226,8 @@ export function useLinkedDoc(options: UseLinkedDocOptions): UseLinkedDocReturn { globalAttachments: [...globalAttachments], markdown: getDocumentMarkdown?.(linkedDoc.filepath, linkedDoc.markdown) ?? linkedDoc.markdown, isConverted: linkedDoc.isConverted, + previousPlan: linkedDoc.previousPlan, + versionInfo: linkedDoc.versionInfo, }); // Update reactive count so button labels can respond let total = 0; @@ -264,6 +323,8 @@ export function useLinkedDoc(options: UseLinkedDocOptions): UseLinkedDocReturn { globalAttachments: [...globalAttachments], markdown: getDocumentMarkdown?.(linkedDoc.filepath, linkedDoc.markdown) ?? linkedDoc.markdown, isConverted: linkedDoc.isConverted, + previousPlan: linkedDoc.previousPlan, + versionInfo: linkedDoc.versionInfo, }); let total = 0; for (const [fp, cached] of docCache.current.entries()) { @@ -294,10 +355,13 @@ export function useLinkedDoc(options: UseLinkedDocOptions): UseLinkedDocReturn { setAnnotations(cached?.annotations ?? []); setGlobalAttachments(cached?.globalAttachments ?? []); setSelectedAnnotationId(null); + const diffBaseline = resolveDiffBaseline(cached, data); setLinkedDoc({ filepath: data.filepath, isConverted: !!data.isConverted, markdown: nextMarkdown, + previousPlan: diffBaseline.previousPlan, + versionInfo: diffBaseline.versionInfo, }); setError(null); sidebar.open(targetTab ?? "toc"); @@ -391,6 +455,8 @@ export function useLinkedDoc(options: UseLinkedDocOptions): UseLinkedDocReturn { globalAttachments: [...globalAttachments], markdown: getDocumentMarkdown?.(linkedDoc.filepath, linkedDoc.markdown) ?? linkedDoc.markdown, isConverted: linkedDoc.isConverted, + previousPlan: linkedDoc.previousPlan, + versionInfo: linkedDoc.versionInfo, }); } @@ -490,5 +556,7 @@ export function useLinkedDoc(options: UseLinkedDocOptions): UseLinkedDocReturn { snapshotSession, restoreSession, docAnnotationCount, + diffPreviousPlan: linkedDoc?.previousPlan ?? null, + diffVersionInfo: linkedDoc?.versionInfo ?? null, }; } From 4dd16eabb9c2e4d159a2177f4a11080dc7f728fd Mon Sep 17 00:00:00 2001 From: Ben Newman <158209000+BenNewman100@users.noreply.github.com> Date: Tue, 21 Jul 2026 18:29:28 -0400 Subject: [PATCH 06/15] feat(editor): render folder-doc version diffs via the active document Folder annotate's version-diff UI (inline PlanDiffViewer blocks, the +N/-M badge, and the Version Browser) was root-document-coupled: usePlanDiff was fed only the root's previousPlan/versionInfo, and every render site keyed off linkedDocHook.isActive to blank out the badge/version tab whenever any linked or folder document was open. Wire the two new per-document seams together instead: - Feed usePlanDiff the active document's own previousPlan/versionInfo/ filepath (falling back to the root document's when none is active), using the document's filepath as usePlanDiff's new docKey so switching documents resets the diff base instead of inheriting the previous one's. - Add per-document fetchers (fetchVersion/fetchVersions with &path=) so selecting a base version or listing versions targets the active document's own history, not the session-bound bare endpoints. - Replace the root-only versionInfo/showVersionsTab reads with the active document's, so the Version Browser now reflects whichever document is on screen (previously it kept showing the root document's versions while a linked doc was open). - Drop the blanket "linkedDocHook.isActive ? null/false : ..." suppression at the Viewer callsite and in DocBadges - planDiffStats/hasPreviousVersion already resolve to the active document's own (possibly absent) diff data, so the badge now shows for folder docs with history and stays hidden for every other document exactly as it did before. Root-document behavior (single-file, plan, review, HTML surfaces) is unaffected: none of those ever set a docKey or have an eligible document history, so they fall through to the same defaults as before. --- packages/editor/App.tsx | 109 ++++++++++++++++++--------- packages/ui/components/DocBadges.tsx | 20 +++-- 2 files changed, 86 insertions(+), 43 deletions(-) diff --git a/packages/editor/App.tsx b/packages/editor/App.tsx index 15f693f9c..bac12f223 100644 --- a/packages/editor/App.tsx +++ b/packages/editor/App.tsx @@ -61,7 +61,7 @@ import { PermissionModeSetup } from '@plannotator/ui/components/PermissionModeSe import { ImageAnnotator } from '@plannotator/ui/components/ImageAnnotator'; import { deriveImageName } from '@plannotator/ui/components/AttachmentsButton'; import { useSidebar, type SidebarTab } from '@plannotator/ui/hooks/useSidebar'; -import { usePlanDiff, type VersionInfo } from '@plannotator/ui/hooks/usePlanDiff'; +import { usePlanDiff, type VersionInfo, type VersionEntry, type PlanDiffFetchers } from '@plannotator/ui/hooks/usePlanDiff'; import { useLinkedDoc, type LinkedDocSessionState } from '@plannotator/ui/hooks/useLinkedDoc'; import { useCodeFilePopout } from '@plannotator/ui/hooks/useCodeFilePopout'; import { useAnnotationDraft, type DraftEditedDocument, type DraftSavedFileChange } from '@plannotator/ui/hooks/useAnnotationDraft'; @@ -686,36 +686,6 @@ const App: React.FC = () => { return () => document.removeEventListener('keydown', handleKeyDown); }, [isPlanDiffActive]); - // Plan diff computation. On the HTML surface the diff is rendered as the real - // page with inline highlights (htmlDiffHtml) instead of the markdown block diff, - // so suppress the markdown diff path there (markdown is empty for HTML). - const planDiff = usePlanDiff( - markdown, - isHtmlSurface ? null : previousPlan, - isHtmlSurface ? null : versionInfo, - ); - const warnFinishEditingFirst = useCallback((target: 'versions' | 'diff') => { - toast('Finish editing first', { - description: target === 'versions' - ? 'Use "Done editing" before changing the comparison version.' - : 'Use "Done editing" before opening the version diff.', - }); - }, []); - const handleSelectBaseVersion = useCallback((version: number) => { - if (isEditingMarkdown) { - warnFinishEditingFirst('versions'); - return Promise.resolve(); - } - return planDiff.selectBaseVersion(version); - }, [isEditingMarkdown, planDiff.selectBaseVersion, warnFinishEditingFirst]); - const handleActivatePlanDiff = useCallback(() => { - if (isEditingMarkdown) { - warnFinishEditingFirst('diff'); - return; - } - setIsPlanDiffActive(true); - }, [isEditingMarkdown, warnFinishEditingFirst]); - const linkedDocSidebar = useMemo(() => ({ ...sidebar, open: openSidebarTab, @@ -789,6 +759,73 @@ const App: React.FC = () => { onAfterBack: restoreLinkedDocumentEditableKey, }); + // Active document's version-diff baseline: the root document's own + // previousPlan/versionInfo (set once from /api/plan) when no linked/folder + // doc is open, or the active document's own baseline when one is — + // captured from its /api/doc response and cached across navigation by + // useLinkedDoc. /api/doc only ever populates these for eligible folder + // files, so any other linked doc naturally resolves to null/null here, + // same as the (now-removed) blanket "linkedDocHook.isActive ? null : ..." + // suppression used to force. + const activeDiffPreviousPlan = linkedDocHook.isActive ? linkedDocHook.diffPreviousPlan : previousPlan; + const activeDiffVersionInfo = linkedDocHook.isActive ? linkedDocHook.diffVersionInfo : versionInfo; + const activeDocFilepath = linkedDocHook.isActive ? linkedDocHook.filepath : null; + + // Per-document version fetchers: only needed while a document with its own + // diff baseline is active (folder annotate) — usePlanDiff's bare-endpoint + // defaults already cover the root document. + const activeDocDiffFetchers = useMemo(() => { + if (!activeDocFilepath) return undefined; + const filepath = activeDocFilepath; + return { + fetchVersion: async (version: number) => { + const res = await fetch(`/api/plan/version?v=${version}&path=${encodeURIComponent(filepath)}`); + if (!res.ok) throw new Error(`Failed to load version ${version}.`); + return (await res.json()) as { plan: string; version: number }; + }, + fetchVersions: async () => { + const res = await fetch(`/api/plan/versions?path=${encodeURIComponent(filepath)}`); + if (!res.ok) throw new Error('Failed to load versions.'); + return (await res.json()) as { project: string; slug: string; versions: VersionEntry[] }; + }, + }; + }, [activeDocFilepath]); + + // Plan diff computation. On the HTML surface the diff is rendered as the real + // page with inline highlights (htmlDiffHtml) instead of the markdown block diff, + // so suppress the markdown diff path there (markdown is empty for HTML). + // `activeDocFilepath` as the docKey resets the diff-base state whenever the + // active document changes, so a newly opened document starts from ITS OWN + // baseline instead of inheriting whatever the previous document had. + const planDiff = usePlanDiff( + markdown, + isHtmlSurface ? null : activeDiffPreviousPlan, + isHtmlSurface ? null : activeDiffVersionInfo, + activeDocDiffFetchers, + activeDocFilepath, + ); + const warnFinishEditingFirst = useCallback((target: 'versions' | 'diff') => { + toast('Finish editing first', { + description: target === 'versions' + ? 'Use "Done editing" before changing the comparison version.' + : 'Use "Done editing" before opening the version diff.', + }); + }, []); + const handleSelectBaseVersion = useCallback((version: number) => { + if (isEditingMarkdown) { + warnFinishEditingFirst('versions'); + return Promise.resolve(); + } + return planDiff.selectBaseVersion(version); + }, [isEditingMarkdown, planDiff.selectBaseVersion, warnFinishEditingFirst]); + const handleActivatePlanDiff = useCallback(() => { + if (isEditingMarkdown) { + warnFinishEditingFirst('diff'); + return; + } + setIsPlanDiffActive(true); + }, [isEditingMarkdown, warnFinishEditingFirst]); + // Keep the early parse-path mirror in sync with the active linked doc so // the blocks/frontmatter memos (declared before this hook) parse with the // right frontmatter rule for the file on screen. @@ -4022,7 +4059,7 @@ const App: React.FC = () => { activeTab={sidebar.activeTab} onToggleTab={toggleSidebarTab} hasDiff={planDiff.hasPreviousVersion} - showVersionsTab={!isHtmlSurface && versionInfo !== null && versionInfo.totalVersions > 1} + showVersionsTab={!isHtmlSurface && activeDiffVersionInfo !== null && activeDiffVersionInfo.totalVersions > 1} showFilesTab={showFilesTab && !archive.archiveMode} showMessagesTab={annotateSource === 'message' && recentMessages.length > 1} showAgentTerminalTab={showAgentTerminalControls} @@ -4082,8 +4119,8 @@ const App: React.FC = () => { onFilesFetchAll={() => fileBrowser.fetchAll(fileBrowserDirs)} onFilesRetryVaultDir={(vaultPath) => fileBrowser.addVaultDir(vaultPath)} hasFileAnnotations={hasFileAnnotations} - showVersionsTab={!isHtmlSurface && versionInfo !== null && versionInfo.totalVersions > 1} - versionInfo={versionInfo} + showVersionsTab={!isHtmlSurface && activeDiffVersionInfo !== null && activeDiffVersionInfo.totalVersions > 1} + versionInfo={activeDiffVersionInfo} versions={planDiff.versions} selectedBaseVersion={planDiff.diffBaseVersion} onSelectBaseVersion={handleSelectBaseVersion} @@ -4392,10 +4429,10 @@ const App: React.FC = () => { onRemoveGlobalAttachment={handleRemoveGlobalAttachment} repoInfo={repoInfo} stickyActions={uiPrefs.stickyActionsEnabled} - planDiffStats={linkedDocHook.isActive ? null : planDiff.diffStats} + planDiffStats={planDiff.diffStats} isPlanDiffActive={isPlanDiffActive} onPlanDiffToggle={() => setIsPlanDiffActive(!isPlanDiffActive)} - hasPreviousVersion={!linkedDocHook.isActive && planDiff.hasPreviousVersion} + hasPreviousVersion={planDiff.hasPreviousVersion} showDemoBadge={!isApiMode && !isLoadingShared && !isSharedSession} maxWidth={annotateReaderMaxWidth} onOpenLinkedDoc={handleOpenLinkedDoc} diff --git a/packages/ui/components/DocBadges.tsx b/packages/ui/components/DocBadges.tsx index 2222800ef..42871c5ce 100644 --- a/packages/ui/components/DocBadges.tsx +++ b/packages/ui/components/DocBadges.tsx @@ -5,9 +5,13 @@ * - layout="column": original location at the top-left of the plan card (absolute) * - layout="row": inside the sticky header lane when the user scrolls * - * In row layout, the demo badge and linked-doc breadcrumb are dropped — the - * sticky lane hides entirely in linked-doc mode, and the demo badge is purely - * decorative top-of-doc context. + * In row layout, the demo badge and linked-doc breadcrumb are dropped — + * everything else is decorative top-of-doc context except the plan-diff + * badge, which is NOT suppressed by linkedDocInfo: `planDiffStats`/ + * `hasPreviousVersion` already arrive pre-scoped to whichever document is + * currently active (root, or a folder/linked doc with its own diff + * baseline), so it renders in linked-doc mode exactly when that document + * actually has a previous version to diff against. */ import React from 'react'; @@ -64,10 +68,12 @@ export const DocBadges: React.FC = ({ ) : null; // In row layout, only PlanDiffBadge (when it has stats to show) and - // archiveInfo actually render — everything else is hidden. Check what - // will truly produce visible output to avoid an empty wrapper div. + // archiveInfo (outside linked-doc mode) actually render — everything else + // is hidden. Check what will truly produce visible output to avoid an + // empty wrapper div. PlanDiffBadge is not linkedDocInfo-gated — see the + // header comment. const anything = isRow - ? (!linkedDocInfo && ((hasPreviousVersion && planDiffStats) || archiveInfo)) + ? (hasPreviousVersion && planDiffStats) || (archiveInfo && !linkedDocInfo) : repoInfo || hasPreviousVersion || showDemoBadge || linkedDocInfo || archiveInfo || sourceInfo || canOpenInApp; if (!anything) return null; @@ -124,7 +130,7 @@ export const DocBadges: React.FC = ({ )} - {onPlanDiffToggle && !linkedDocInfo && ( + {onPlanDiffToggle && ( Date: Tue, 21 Jul 2026 19:01:18 -0400 Subject: [PATCH 07/15] feat(pi): extend per-file version history to folder annotate sessions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mirrors the Bun runtime's folder annotate history support (packages/server/annotate.ts + reference-handlers.ts) in the Pi Node server: - Vendor the shared annotate-history helper (deriveAnnotateHistorySlug, computeAnnotateHistory) from packages/shared into generated/ via vendor.sh, and delegate the single-file version-history pipeline in serverAnnotate.ts to it instead of the hand-duplicated inline block. Behavior for single-file sessions is unchanged. - Eligible folder files served through /api/doc now get snapshotted into the same version history the single-file flow uses, and their doc responses carry the same previousPlan/versionInfo/diffCurrent fields /api/plan already returns. The pipeline runs lazily on first open and is memoized per resolved absolute path for the life of the server, so reopening a file never re-snapshots it. - /api/plan/version and /api/plan/versions gain an optional path (+ base) query param so folder sessions can ask for a specific file's history; the slug is always derived server-side from the resolved, containment-checked path (resolveAllowedDocPath in reference.ts) — never accepted from the client. --- apps/pi-extension/server/reference.ts | 95 ++++++++++++++-- apps/pi-extension/server/serverAnnotate.ts | 121 +++++++++++++-------- apps/pi-extension/vendor.sh | 2 +- 3 files changed, 163 insertions(+), 55 deletions(-) diff --git a/apps/pi-extension/server/reference.ts b/apps/pi-extension/server/reference.ts index 15cee920f..6da235a15 100644 --- a/apps/pi-extension/server/reference.ts +++ b/apps/pi-extension/server/reference.ts @@ -50,16 +50,38 @@ import { readSourceFileSnapshot, resolveExistingSourceSaveFile, } from "../generated/source-save-node.js"; +import type { AnnotateHistoryResult } from "../generated/annotate-history.js"; import { preloadFile } from "@pierre/diffs/ssr"; type Res = ServerResponse; +/** + * Extensions eligible for annotate's per-file version history when served + * through /api/doc (folder annotate mode). Deliberately narrower than the + * full annotatable-text set: markdown-branch documents only, matching the + * gate the folder annotate design settled on. Mirrors packages/server/reference-handlers.ts. + */ +const ANNOTATE_HISTORY_ELIGIBLE_REGEX = /\.(md|txt)$/i; + export interface HandleDocOptions { rewriteHtml?: (html: string, filepath: string) => string; sourceSaveFilePath?: string; sourceSaveFolderPath?: string; onSourceDocumentServed?: (path: string) => void; rootPaths?: string[]; + /** + * When set, /api/doc runs annotate's per-file version-history pipeline for + * eligible markdown-branch documents (local file under an allowed root, + * .md/.txt, not HTML, not a converted doc, under the annotatable size cap + * already enforced above) and merges `previousPlan`/`versionInfo`/ + * `diffCurrent` into the response — the same field names the single-file + * /api/plan payload uses. `compute` is expected to memoize per resolved + * path itself (the annotate server keys its cache by path in its own + * closure); this module only decides *whether* to call it. + */ + annotateHistory?: { + compute: (resolvedFilePath: string, content: string) => AnnotateHistoryResult | null; + }; } interface HandleDocExistsOptions { @@ -96,6 +118,33 @@ function getTrustedBaseDir(base: string | null, roots: string[]): string | null return isWithinAllowedRoots(resolvedBase, roots) ? resolvedBase : null; } +export type ResolveAllowedDocPathResult = + | { kind: "resolved"; path: string } + | { kind: "denied" }; + +/** + * Resolve a client-supplied path the same way /api/doc's base-relative and + * absolute branches do (see `getTrustedBaseDir` / `isWithinAllowedRoots` + * above), for callers that need the canonical contained path without + * reading the file — namely the annotate version endpoints, which derive a + * history slug from the resolved path rather than trusting a client-supplied + * slug (a client slug would be a path-traversal vector: the history dir + * lookup joins it into a filesystem path unsanitized). Mirrors + * packages/server/reference-handlers.ts. + */ +export function resolveAllowedDocPath( + requestedPath: string, + base: string | null, + options?: { rootPaths?: string[] }, +): ResolveAllowedDocPathResult { + const allowedRoots = getAllowedRootPaths(options); + const resolvedBase = getTrustedBaseDir(base, allowedRoots); + const candidate = resolveUserPath(requestedPath, resolvedBase ?? undefined); + return isWithinAllowedRoots(candidate, allowedRoots) + ? { kind: "resolved", path: candidate } + : { kind: "denied" }; +} + function relativizeToAllowedRoots(path: string, roots: string[]): string { for (const root of roots) { const prefix = `${root}/`; @@ -160,11 +209,18 @@ function resolveMarkdownFileFromAllowedRoots(input: string, roots: string[]): Ro return { kind: "not_found", input }; } +type DocOptionsResult = T & { + sourceSave?: SourceSaveCapability; + previousPlan?: string | null; + versionInfo?: AnnotateHistoryResult["versionInfo"]; + diffCurrent?: string; +}; + function applyDocOptions>( data: T, options: HandleDocOptions = {}, sourceSnapshot?: SourceFileSnapshot, -): T & { sourceSave?: SourceSaveCapability } { +): DocOptionsResult { const next: Record = { ...data }; if ( typeof next.rawHtml === "string" && @@ -173,16 +229,37 @@ function applyDocOptions>( ) { next.rawHtml = options.rewriteHtml(next.rawHtml, next.filepath); } + // Annotate version history (folder mode only — see HandleDocOptions.annotateHistory). + // Independent of the sourceSave branching below: only markdown-branch + // documents (not HTML, not converted) with a .md/.txt extension are + // eligible. The 2MB annotatable-file size cap is already enforced by the + // caller before any of these responses are built, so no separate check is + // needed here. Mirrors packages/server/reference-handlers.ts. + if ( + options.annotateHistory && + typeof data.filepath === "string" && + data.renderAs === "markdown" && + data.isConverted !== true && + typeof data.markdown === "string" && + ANNOTATE_HISTORY_ELIGIBLE_REGEX.test(data.filepath) + ) { + const history = options.annotateHistory.compute(data.filepath, data.markdown); + if (history) { + next.previousPlan = history.previousPlan; + next.versionInfo = history.versionInfo; + next.diffCurrent = history.diffCurrent; + } + } if (typeof data.filepath !== "string") { - return options.sourceSaveFolderPath || options.sourceSaveFilePath - ? { ...next, sourceSave: disabledSourceSave("not-local-file") } as T & { sourceSave?: SourceSaveCapability } - : next as T & { sourceSave?: SourceSaveCapability }; + return (options.sourceSaveFolderPath || options.sourceSaveFilePath + ? { ...next, sourceSave: disabledSourceSave("not-local-file") } + : next) as DocOptionsResult; } if (data.renderAs === "html") { - return { ...next, sourceSave: disabledSourceSave("html-render") } as T & { sourceSave?: SourceSaveCapability }; + return { ...next, sourceSave: disabledSourceSave("html-render") } as DocOptionsResult; } if (data.isConverted === true) { - return { ...next, sourceSave: disabledSourceSave("converted-source") } as T & { sourceSave?: SourceSaveCapability }; + return { ...next, sourceSave: disabledSourceSave("converted-source") } as DocOptionsResult; } if (options.sourceSaveFilePath) { const sourcePath = resolveExistingSourceSaveFile("single-file", options.sourceSaveFilePath); @@ -191,10 +268,10 @@ function applyDocOptions>( : createSourceSaveCapability("single-file", data.filepath); if (sourcePath && doc.enabled && sourcePath === doc.path) { options.onSourceDocumentServed?.(doc.path); - return { ...next, sourceSave: doc } as T & { sourceSave?: SourceSaveCapability }; + return { ...next, sourceSave: doc } as DocOptionsResult; } } - if (!options.sourceSaveFolderPath) return next as T & { sourceSave?: SourceSaveCapability }; + if (!options.sourceSaveFolderPath) return next as DocOptionsResult; const sourceSave = sourceSnapshot ? createSourceSaveCapabilityFromSnapshot("folder-file", data.filepath, sourceSnapshot, options.sourceSaveFolderPath) : createSourceSaveCapability("folder-file", data.filepath, options.sourceSaveFolderPath); @@ -202,7 +279,7 @@ function applyDocOptions>( return { ...next, sourceSave, - } as T & { sourceSave?: SourceSaveCapability }; + } as DocOptionsResult; } function jsonDoc( diff --git a/apps/pi-extension/server/serverAnnotate.ts b/apps/pi-extension/server/serverAnnotate.ts index 1b0f5503b..9099b6e37 100644 --- a/apps/pi-extension/server/serverAnnotate.ts +++ b/apps/pi-extension/server/serverAnnotate.ts @@ -4,7 +4,8 @@ import { existsSync, readFileSync, statSync } from "node:fs"; import { randomUUID } from "node:crypto"; import { contentHash, deleteDraft } from "../generated/draft.js"; -import { saveToHistory, getPlanVersion, getVersionCount, listVersions } from "../generated/storage.js"; +import { getPlanVersion, getVersionCount, listVersions } from "../generated/storage.js"; +import { computeAnnotateHistory, deriveAnnotateHistorySlug, type AnnotateHistoryResult } from "../generated/annotate-history.js"; import { htmlDiff } from "../generated/html-diff.js"; import { saveConfig, detectGitUser, getServerConfig, loadConfig, resolveSharingEnabled, resolveAnnotateHistory } from "../generated/config.js"; import { disabledSourceSave, type SourceSaveRequest } from "../generated/source-save.js"; @@ -41,6 +42,7 @@ import { handleObsidianVaultsRequest, handleObsidianFilesRequest, handleObsidianDocRequest, + resolveAllowedDocPath, } from "./reference.js"; import { handleFileBrowserStreamRequest } from "./file-browser-watch.js"; import { resolveUserPath, warmFileListCache } from "../generated/resolve-file.js"; @@ -220,56 +222,37 @@ export async function startAnnotateServer(options: { // when headings change. Diff content is the markdown, or the raw HTML source // when rendering HTML. Only single local files (not URLs/folders/messages). const annotateProjectName = options.project ?? "_unknown"; - let annotateHistory: - | { - slug: string; - diffCurrent: string; - previousPlan: string | null; - versionInfo: { version: number; totalVersions: number; project: string }; - } - | null = null; + const annotateHistoryEnabled = resolveAnnotateHistory(loadConfig()); + let annotateHistory: AnnotateHistoryResult | null = null; { const historyContent = options.renderHtml && options.rawHtml ? options.rawHtml : options.markdown; const eligible = (options.mode || "annotate") === "annotate" && !/^https?:\/\//i.test(options.filePath) && historyContent.length > 0 && - resolveAnnotateHistory(loadConfig()); + annotateHistoryEnabled; + // History is an enhancement, never a gate: a read-only/full data dir + // must degrade to stateless annotate (no version diff), not fail the + // whole session before the UI ever opens. (computeAnnotateHistory never + // throws — it logs and returns null on any storage error.) if (eligible) { - const base = - (options.filePath.split(/[\\/]/).pop() || "document") - .toLowerCase() - .replace(/[^a-z0-9]+/g, "-") - .replace(/^-+|-+$/g, "") - .slice(0, 60) || "document"; - const slug = `annotate-${base}-${contentHash(resolvePath(options.filePath)).slice(0, 8)}`; - // History is an enhancement, never a gate: a read-only/full data dir - // must degrade to stateless annotate (no version diff), not fail the - // whole session before the UI ever opens. Mirrors packages/server. - try { - const saved = saveToHistory(annotateProjectName, slug, historyContent); - const previousPlan = - saved.version > 1 - ? getPlanVersion(annotateProjectName, slug, saved.version - 1) - : null; - annotateHistory = { - slug, - diffCurrent: historyContent, - previousPlan, - versionInfo: { - version: saved.version, - totalVersions: getVersionCount(annotateProjectName, slug), - project: annotateProjectName, - }, - }; - } catch (error) { - console.error( - `[plannotator] warning: annotate history unavailable (${error instanceof Error ? error.message : String(error)}); continuing without version diff`, - ); - } + annotateHistory = computeAnnotateHistory(annotateProjectName, resolvePath(options.filePath), historyContent); } } + // Folder annotate: the same per-file version history, but run lazily the + // first time a folder file is opened via /api/doc (not eagerly for every + // file in the folder) and memoized per resolved absolute path for the life + // of this server — reopening the same file in this session never re-snapshots. + const folderAnnotateHistoryCache = new Map(); + function computeFolderAnnotateHistory(resolvedFilePath: string, content: string): AnnotateHistoryResult | null { + const cached = folderAnnotateHistoryCache.get(resolvedFilePath); + if (cached !== undefined) return cached; + const result = computeAnnotateHistory(annotateProjectName, resolvedFilePath, content); + folderAnnotateHistoryCache.set(resolvedFilePath, result); + return result; + } + // Detect repo info (cached for this session) const repoInfo = getRepoInfo(); @@ -438,9 +421,34 @@ export async function startAnnotateServer(options: { }); } else if (url.pathname === "/api/plan/version" && req.method === "GET") { // fetch a specific version of the annotated file (version diff base picker) - if (!annotateHistory) { - json(res, { error: "No version history" }, 404); - return; + // + // Folder sessions pass `?path=` (optionally `&base=`) to identify which + // file's history to read, resolved and containment-checked exactly like + // /api/doc; the slug is always derived server-side from that resolved + // path — a client-supplied slug is never accepted, since the history + // lookup joins it into a filesystem path unsanitized. Without `path`, + // behavior is unchanged: the single session's own history is used. + const pathParam = url.searchParams.get("path"); + let slug: string; + if (pathParam !== null) { + const resolved = resolveAllowedDocPath(pathParam, url.searchParams.get("base"), { + rootPaths: getReferenceRootPaths(), + }); + if (resolved.kind === "denied") { + json(res, { error: "Access denied: path is outside project root" }, 403); + return; + } + slug = deriveAnnotateHistorySlug(resolved.path); + if (getVersionCount(annotateProjectName, slug) === 0) { + json(res, { error: "No version history" }, 404); + return; + } + } else { + if (!annotateHistory) { + json(res, { error: "No version history" }, 404); + return; + } + slug = annotateHistory.slug; } const vParam = url.searchParams.get("v"); const v = vParam ? parseInt(vParam, 10) : NaN; @@ -448,7 +456,7 @@ export async function startAnnotateServer(options: { json(res, { error: "Invalid version number" }, 400); return; } - const content = getPlanVersion(annotateProjectName, annotateHistory.slug, v); + const content = getPlanVersion(annotateProjectName, slug, v); if (content === null) { json(res, { error: "Version not found" }, 404); return; @@ -456,6 +464,25 @@ export async function startAnnotateServer(options: { json(res, { plan: content, version: v }); } else if (url.pathname === "/api/plan/versions" && req.method === "GET") { // list all stored versions of the annotated file (Version Browser) + // Same `?path=`/`&base=` parameterization as /api/plan/version above. + const pathParam = url.searchParams.get("path"); + if (pathParam !== null) { + const resolved = resolveAllowedDocPath(pathParam, url.searchParams.get("base"), { + rootPaths: getReferenceRootPaths(), + }); + if (resolved.kind === "denied") { + json(res, { error: "Access denied: path is outside project root" }, 403); + return; + } + const slug = deriveAnnotateHistorySlug(resolved.path); + const versions = listVersions(annotateProjectName, slug); + json(res, { + project: annotateProjectName, + slug: versions.length > 0 ? slug : null, + versions, + }); + return; + } if (!annotateHistory) { json(res, { project: annotateProjectName, slug: null, versions: [] }); return; @@ -543,6 +570,10 @@ export async function startAnnotateServer(options: { sourceSaveFolderPath: options.mode === "annotate-folder" ? options.folderPath : undefined, onSourceDocumentServed: (path) => openedSourceFilePaths.add(path), rootPaths: getReferenceRootPaths(), + annotateHistory: + options.mode === "annotate-folder" && annotateHistoryEnabled + ? { compute: computeFolderAnnotateHistory } + : undefined, }); } else if (url.pathname === "/api/source/save" && req.method === "POST") { let body: SourceSaveRequest; diff --git a/apps/pi-extension/vendor.sh b/apps/pi-extension/vendor.sh index 696c5c338..2dc217494 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 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 commit-avatars commit-history port-range; 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 commit-avatars commit-history port-range; do src="../../packages/shared/$f.ts" printf '// @generated — DO NOT EDIT. Source: packages/shared/%s.ts\n' "$f" | cat - "$src" > "generated/$f.ts" done From 9e4556e6034811ebfa2695c67a1a39305661a9a0 Mon Sep 17 00:00:00 2001 From: Ben Newman <158209000+BenNewman100@users.noreply.github.com> Date: Tue, 21 Jul 2026 19:01:29 -0400 Subject: [PATCH 08/15] test(pi): cover folder annotate version history Adds apps/pi-extension/server/annotate-history.test.ts, the Node mirror of packages/server/annotate.test.ts's folder-history describe block: first-open snapshot + same-session memoization, cross-mode slug continuity with the single-file flow, the config toggle, an ineligible (HTML) file type, degrade-on-unwritable-history-dir, and the path-parameterized version endpoints (including containment rejection and the no-path fallback). History writes land in the real ~/.plannotator data dir rather than a per-test PLANNOTATOR_DATA_DIR override: generated/storage.js caches its data directory in a module-level constant at first import, so a per-test env var override taken after that point silently no-ops. Each test uses its own unique project namespace instead, same approach as the Bun-side suite. --- .../server/annotate-history.test.ts | 306 ++++++++++++++++++ 1 file changed, 306 insertions(+) create mode 100644 apps/pi-extension/server/annotate-history.test.ts diff --git a/apps/pi-extension/server/annotate-history.test.ts b/apps/pi-extension/server/annotate-history.test.ts new file mode 100644 index 000000000..df152d7e1 --- /dev/null +++ b/apps/pi-extension/server/annotate-history.test.ts @@ -0,0 +1,306 @@ +/** + * Annotate server (Pi/Node): folder annotate version history + * + * Node mirror of packages/server/annotate.test.ts's "folder annotate history" + * describe block — exercises the same lazy, memoized per-file history + * pipeline wired into apps/pi-extension/server/serverAnnotate.ts's /api/doc + * route, and the path-parameterized /api/plan/version(s) endpoints. + * + * History writes go to the real ~/.plannotator data dir (or PLANNOTATOR_DATA_DIR + * if the environment already had it set before this process's first import of + * generated/storage.js): that module caches its data directory in a + * module-level constant at import time, so a per-test PLANNOTATOR_DATA_DIR + * override taken here would silently no-op once another test file has already + * imported it. Each test uses its own unique project namespace instead (same + * approach as the Bun-side suite) so runs never collide. + */ + +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { mkdirSync, mkdtempSync, realpathSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { startAnnotateServer } from "./serverAnnotate"; +import { deriveAnnotateHistorySlug } from "../generated/annotate-history.js"; +import { getPlannotatorDataDir } from "../generated/data-dir.js"; + +describe("pi annotate server: folder annotate history", () => { + let savedPort: string | undefined; + let savedRemote: string | undefined; + let savedHistoryFlag: string | undefined; + + beforeEach(() => { + savedPort = process.env.PLANNOTATOR_PORT; + savedRemote = process.env.PLANNOTATOR_REMOTE; + savedHistoryFlag = process.env.PLANNOTATOR_ANNOTATE_HISTORY; + delete process.env.PLANNOTATOR_PORT; + process.env.PLANNOTATOR_REMOTE = "0"; + // Force the toggle on for every test but the one that explicitly flips it + // off — a real ~/.plannotator/config.json on the machine running these + // tests must never change the outcome. + process.env.PLANNOTATOR_ANNOTATE_HISTORY = "1"; + }); + + afterEach(() => { + if (savedPort === undefined) delete process.env.PLANNOTATOR_PORT; + else process.env.PLANNOTATOR_PORT = savedPort; + if (savedRemote === undefined) delete process.env.PLANNOTATOR_REMOTE; + else process.env.PLANNOTATOR_REMOTE = savedRemote; + if (savedHistoryFlag === undefined) delete process.env.PLANNOTATOR_ANNOTATE_HISTORY; + else process.env.PLANNOTATOR_ANNOTATE_HISTORY = savedHistoryFlag; + }); + + function uniqueProject(label: string): string { + return `_pi_annotate_history_test_${label}_${Date.now()}_${Math.random().toString(36).slice(2, 8)}`; + } + + test("first open mints one version; reopening in the same session is memoized (no re-snapshot even if the file changes on disk)", async () => { + const folderPath = mkdtempSync(join(tmpdir(), "plannotator-pi-folder-history-first-open-")); + const docPath = join(folderPath, "note.md"); + writeFileSync(docPath, "V1\n", "utf-8"); + const project = uniqueProject("first-open"); + + const server = await startAnnotateServer({ + markdown: "", + filePath: folderPath, + folderPath, + mode: "annotate-folder", + htmlContent: "", + project, + }); + + try { + const first = await fetch(`${server.url}/api/doc?path=${encodeURIComponent(docPath)}`); + const firstJson = (await first.json()) as { + markdown?: string; + previousPlan?: string | null; + versionInfo?: { version: number; totalVersions: number; project: string }; + diffCurrent?: string; + }; + expect(firstJson.markdown).toBe("V1\n"); + expect(firstJson.previousPlan).toBeNull(); + expect(firstJson.versionInfo).toEqual({ version: 1, totalVersions: 1, project }); + expect(firstJson.diffCurrent).toBe("V1\n"); + + // Change the file on disk between opens — a re-run of the pipeline + // would mint version 2. Memoization must prevent that. + writeFileSync(docPath, "V2\n", "utf-8"); + + const second = await fetch(`${server.url}/api/doc?path=${encodeURIComponent(docPath)}`); + const secondJson = (await second.json()) as { + markdown?: string; + previousPlan?: string | null; + versionInfo?: { version: number; totalVersions: number; project: string }; + }; + // The live document content is always read fresh from disk... + expect(secondJson.markdown).toBe("V2\n"); + // ...but the history snapshot/diff fields stay exactly what first-open computed. + expect(secondJson.previousPlan).toBeNull(); + expect(secondJson.versionInfo).toEqual({ version: 1, totalVersions: 1, project }); + + const versions = await fetch(`${server.url}/api/plan/versions?path=${encodeURIComponent(docPath)}`); + const versionsJson = (await versions.json()) as { versions: unknown[] }; + expect(versionsJson.versions).toHaveLength(1); + } finally { + server.stop(); + } + }); + + test("cross-mode slug continuity: a version saved via single-file flow is served as the baseline when a folder session opens the same path", async () => { + const folderPath = mkdtempSync(join(tmpdir(), "plannotator-pi-folder-history-cross-mode-")); + const docPath = join(folderPath, "note.md"); + const project = uniqueProject("cross-mode"); + + // Single-file session saves "V1" as version 1 for this exact resolved path. + const seedServer = await startAnnotateServer({ + markdown: "V1\n", + filePath: docPath, + htmlContent: "", + mode: "annotate", + project, + }); + seedServer.stop(); + + // The folder session reads different content off disk, so it mints version 2. + writeFileSync(docPath, "V2\n", "utf-8"); + + const server = await startAnnotateServer({ + markdown: "", + filePath: folderPath, + folderPath, + mode: "annotate-folder", + htmlContent: "", + project, + }); + + try { + const response = await fetch(`${server.url}/api/doc?path=${encodeURIComponent(docPath)}`); + const json = (await response.json()) as { + previousPlan?: string | null; + versionInfo?: { version: number; totalVersions: number; project: string }; + }; + expect(json.previousPlan).toBe("V1\n"); + expect(json.versionInfo).toEqual({ version: 2, totalVersions: 2, project }); + + const versionOne = await fetch(`${server.url}/api/plan/version?path=${encodeURIComponent(docPath)}&v=1`); + const versionOneJson = (await versionOne.json()) as { plan?: string }; + expect(versionOneJson.plan).toBe("V1\n"); + } finally { + server.stop(); + } + }); + + test("config toggle off: no snapshot, no diff fields, doc still serves", async () => { + process.env.PLANNOTATOR_ANNOTATE_HISTORY = "0"; + const folderPath = mkdtempSync(join(tmpdir(), "plannotator-pi-folder-history-toggle-off-")); + const docPath = join(folderPath, "note.md"); + writeFileSync(docPath, "Content\n", "utf-8"); + const project = uniqueProject("toggle-off"); + + const server = await startAnnotateServer({ + markdown: "", + filePath: folderPath, + folderPath, + mode: "annotate-folder", + htmlContent: "", + project, + }); + + try { + const response = await fetch(`${server.url}/api/doc?path=${encodeURIComponent(docPath)}`); + expect(response.status).toBe(200); + const json = (await response.json()) as Record; + expect(json.markdown).toBe("Content\n"); + expect("previousPlan" in json).toBe(false); + expect("versionInfo" in json).toBe(false); + expect("diffCurrent" in json).toBe(false); + + const versions = await fetch(`${server.url}/api/plan/versions?path=${encodeURIComponent(docPath)}`); + const versionsJson = (await versions.json()) as { slug: string | null; versions: unknown[] }; + expect(versionsJson).toEqual({ project, slug: null, versions: [] }); + } finally { + server.stop(); + } + }); + + test("ineligible file type (HTML) serves as today with no snapshot", async () => { + const folderPath = mkdtempSync(join(tmpdir(), "plannotator-pi-folder-history-html-")); + const docPath = join(folderPath, "page.html"); + writeFileSync(docPath, "Hi", "utf-8"); + const project = uniqueProject("html"); + + const server = await startAnnotateServer({ + markdown: "", + filePath: folderPath, + folderPath, + mode: "annotate-folder", + htmlContent: "", + project, + }); + + try { + const response = await fetch(`${server.url}/api/doc?path=${encodeURIComponent(docPath)}`); + expect(response.status).toBe(200); + const json = (await response.json()) as Record; + expect(json.renderAs).toBe("html"); + expect("previousPlan" in json).toBe(false); + expect("versionInfo" in json).toBe(false); + expect("diffCurrent" in json).toBe(false); + + const versions = await fetch(`${server.url}/api/plan/versions?path=${encodeURIComponent(docPath)}`); + const versionsJson = (await versions.json()) as { slug: string | null; versions: unknown[] }; + expect(versionsJson).toEqual({ project, slug: null, versions: [] }); + } finally { + server.stop(); + } + }); + + test("an unwritable history directory degrades to a plain render, no error propagates", async () => { + const folderPath = mkdtempSync(join(tmpdir(), "plannotator-pi-folder-history-unwritable-")); + const docPath = join(folderPath, "note.md"); + writeFileSync(docPath, "Content\n", "utf-8"); + const project = uniqueProject("unwritable"); + + // Block the exact history directory the pipeline will try to mkdir by + // pre-creating a plain FILE at that path — mkdirSync(recursive) throws + // when a target segment exists and is not a directory, on every platform. + const slug = deriveAnnotateHistorySlug(docPath); + const historyProjectDir = join(getPlannotatorDataDir(), "history", project); + mkdirSync(historyProjectDir, { recursive: true }); + writeFileSync(join(historyProjectDir, slug), "not a directory", "utf-8"); + + const server = await startAnnotateServer({ + markdown: "", + filePath: folderPath, + folderPath, + mode: "annotate-folder", + htmlContent: "", + project, + }); + + try { + const response = await fetch(`${server.url}/api/doc?path=${encodeURIComponent(docPath)}`); + expect(response.status).toBe(200); + const json = (await response.json()) as Record; + expect(json.markdown).toBe("Content\n"); + expect("previousPlan" in json).toBe(false); + expect("versionInfo" in json).toBe(false); + expect("diffCurrent" in json).toBe(false); + } finally { + server.stop(); + } + }); + + test("version endpoints: path param serves that file's versions; without path, single-session binding is unchanged; out-of-root path is rejected", async () => { + const folderPath = mkdtempSync(join(tmpdir(), "plannotator-pi-folder-history-endpoints-")); + const docPath = join(folderPath, "note.md"); + writeFileSync(docPath, "V1\n", "utf-8"); + const project = uniqueProject("endpoints"); + + const server = await startAnnotateServer({ + markdown: "", + filePath: folderPath, + folderPath, + mode: "annotate-folder", + htmlContent: "", + project, + }); + + try { + // No history yet for this path: version endpoints report empty, not an error. + const versionsBefore = await fetch(`${server.url}/api/plan/versions?path=${encodeURIComponent(docPath)}`); + expect(await versionsBefore.json()).toEqual({ project, slug: null, versions: [] }); + const versionBefore = await fetch(`${server.url}/api/plan/version?path=${encodeURIComponent(docPath)}&v=1`); + expect(versionBefore.status).toBe(404); + expect(await versionBefore.json()).toEqual({ error: "No version history" }); + + // Open the file so history is initialized this session. + await fetch(`${server.url}/api/doc?path=${encodeURIComponent(docPath)}`); + + const versionsAfter = await fetch(`${server.url}/api/plan/versions?path=${encodeURIComponent(docPath)}`); + const versionsAfterJson = (await versionsAfter.json()) as { slug: string | null; versions: { version: number }[] }; + expect(versionsAfterJson.slug).not.toBeNull(); + expect(versionsAfterJson.versions).toHaveLength(1); + + const versionAfter = await fetch(`${server.url}/api/plan/version?path=${encodeURIComponent(docPath)}&v=1`); + expect(versionAfter.status).toBe(200); + expect(await versionAfter.json()).toEqual({ plan: "V1\n", version: 1 }); + + // A path outside the folder root is rejected the same way /api/doc rejects it. + const outsidePath = join(realpathSync(tmpdir()), "outside.md"); + const deniedVersions = await fetch(`${server.url}/api/plan/versions?path=${encodeURIComponent(outsidePath)}`); + expect(deniedVersions.status).toBe(403); + const deniedVersion = await fetch(`${server.url}/api/plan/version?path=${encodeURIComponent(outsidePath)}&v=1`); + expect(deniedVersion.status).toBe(403); + + // Without a path param at all, behavior is exactly today's: this + // session has no single-file annotateHistory binding (it's a folder + // session), so both endpoints report "no history" as before. + const noPathVersions = await fetch(`${server.url}/api/plan/versions`); + expect(await noPathVersions.json()).toEqual({ project, slug: null, versions: [] }); + const noPathVersion = await fetch(`${server.url}/api/plan/version?v=1`); + expect(noPathVersion.status).toBe(404); + } finally { + server.stop(); + } + }); +}); From 0bf486ba17846444d78a3e263e8ea693114a350c Mon Sep 17 00:00:00 2001 From: Ben Newman <158209000+BenNewman100@users.noreply.github.com> Date: Tue, 21 Jul 2026 19:39:14 -0400 Subject: [PATCH 09/15] ci: run docKey/linked-doc DOM tests in CI usePlanDiff.test.tsx and useLinkedDoc.test.tsx use the test.skipIf(!hasDom) pattern but were never added to the DOM_TESTS step, so they silently skipped under CI's plain `bun test` and never actually ran. --- .github/workflows/test.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 6b9c646c2..2e2732ee8 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -63,6 +63,8 @@ jobs: packages/ui/hooks/useExternalAnnotations.seam.test.tsx packages/ui/hooks/useAIChat.seam.test.tsx packages/ui/hooks/useFileBrowser.seam.test.tsx + packages/ui/hooks/usePlanDiff.test.tsx + packages/ui/hooks/useLinkedDoc.test.tsx pi-extension-ai-runtime-windows: # Exercises the Pi extension's Node/jiti server mirror on Windows with an From db89202fb5fc49827108e1eec61aaebd9fb0dae6 Mon Sep 17 00:00:00 2001 From: Ben Newman <158209000+BenNewman100@users.noreply.github.com> Date: Tue, 21 Jul 2026 19:39:37 -0400 Subject: [PATCH 10/15] refactor(annotate): drop diffCurrent from the folder /api/doc path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit diffCurrent equals the document's own markdown and the client never reads it off /api/doc — it only exists on /api/plan for legacy single-file shape parity, which is untouched. Stop merging it into folder /api/doc responses and stop retaining it in the per-launch folder history memo (Bun and Pi), and drop the now-unused field from LinkedDocLoadData. - packages/server/reference-handlers.ts: new FolderAnnotateHistory type (AnnotateHistoryResult minus diffCurrent); applyDocOptions no longer copies diffCurrent onto the response - packages/server/annotate.ts: the folder memo now stores/returns only slug/previousPlan/versionInfo - apps/pi-extension/server/reference.ts + serverAnnotate.ts: mirrored changes for the Pi runtime - packages/ui/hooks/useLinkedDoc.ts: removed the unused diffCurrent field from LinkedDocLoadData --- apps/pi-extension/server/reference.ts | 26 +++++++++++++++------- apps/pi-extension/server/serverAnnotate.ts | 13 ++++++++--- packages/server/annotate.ts | 14 ++++++++---- packages/server/reference-handlers.ts | 25 ++++++++++++++------- packages/ui/hooks/useLinkedDoc.ts | 3 --- 5 files changed, 55 insertions(+), 26 deletions(-) diff --git a/apps/pi-extension/server/reference.ts b/apps/pi-extension/server/reference.ts index 6da235a15..77f2b9caa 100644 --- a/apps/pi-extension/server/reference.ts +++ b/apps/pi-extension/server/reference.ts @@ -53,6 +53,16 @@ import { import type { AnnotateHistoryResult } from "../generated/annotate-history.js"; import { preloadFile } from "@pierre/diffs/ssr"; +/** + * Subset of AnnotateHistoryResult the folder /api/doc path actually needs. + * `diffCurrent` is omitted: it always equals the request's own `content` and + * the client never reads it off this response (the single-file /api/plan + * payload still returns the full AnnotateHistoryResult, `diffCurrent` + * included, for legacy shape parity — see serverAnnotate.ts). Mirrors + * packages/server/reference-handlers.ts. + */ +export type FolderAnnotateHistory = Omit; + type Res = ServerResponse; /** @@ -73,14 +83,16 @@ export interface HandleDocOptions { * When set, /api/doc runs annotate's per-file version-history pipeline for * eligible markdown-branch documents (local file under an allowed root, * .md/.txt, not HTML, not a converted doc, under the annotatable size cap - * already enforced above) and merges `previousPlan`/`versionInfo`/ - * `diffCurrent` into the response — the same field names the single-file - * /api/plan payload uses. `compute` is expected to memoize per resolved - * path itself (the annotate server keys its cache by path in its own - * closure); this module only decides *whether* to call it. + * already enforced above) and merges `previousPlan`/`versionInfo` into the + * response — the same field names the single-file /api/plan payload uses + * (which additionally returns `diffCurrent`; the folder path omits it + * since it always equals the document's own content and the client never + * reads it). `compute` is expected to memoize per resolved path itself + * (the annotate server keys its cache by path in its own closure); this + * module only decides *whether* to call it. */ annotateHistory?: { - compute: (resolvedFilePath: string, content: string) => AnnotateHistoryResult | null; + compute: (resolvedFilePath: string, content: string) => FolderAnnotateHistory | null; }; } @@ -213,7 +225,6 @@ type DocOptionsResult = T & { sourceSave?: SourceSaveCapability; previousPlan?: string | null; versionInfo?: AnnotateHistoryResult["versionInfo"]; - diffCurrent?: string; }; function applyDocOptions>( @@ -247,7 +258,6 @@ function applyDocOptions>( if (history) { next.previousPlan = history.previousPlan; next.versionInfo = history.versionInfo; - next.diffCurrent = history.diffCurrent; } } if (typeof data.filepath !== "string") { diff --git a/apps/pi-extension/server/serverAnnotate.ts b/apps/pi-extension/server/serverAnnotate.ts index 9099b6e37..74109fbf0 100644 --- a/apps/pi-extension/server/serverAnnotate.ts +++ b/apps/pi-extension/server/serverAnnotate.ts @@ -43,6 +43,7 @@ import { handleObsidianFilesRequest, handleObsidianDocRequest, resolveAllowedDocPath, + type FolderAnnotateHistory, } from "./reference.js"; import { handleFileBrowserStreamRequest } from "./file-browser-watch.js"; import { resolveUserPath, warmFileListCache } from "../generated/resolve-file.js"; @@ -244,11 +245,17 @@ export async function startAnnotateServer(options: { // first time a folder file is opened via /api/doc (not eagerly for every // file in the folder) and memoized per resolved absolute path for the life // of this server — reopening the same file in this session never re-snapshots. - const folderAnnotateHistoryCache = new Map(); - function computeFolderAnnotateHistory(resolvedFilePath: string, content: string): AnnotateHistoryResult | null { + // The memo drops `diffCurrent` (it always equals the request's own content + // and the client never reads it off /api/doc) — only slug/previousPlan/ + // versionInfo are retained. + const folderAnnotateHistoryCache = new Map(); + function computeFolderAnnotateHistory(resolvedFilePath: string, content: string): FolderAnnotateHistory | null { const cached = folderAnnotateHistoryCache.get(resolvedFilePath); if (cached !== undefined) return cached; - const result = computeAnnotateHistory(annotateProjectName, resolvedFilePath, content); + const full = computeAnnotateHistory(annotateProjectName, resolvedFilePath, content); + const result: FolderAnnotateHistory | null = full + ? { slug: full.slug, previousPlan: full.previousPlan, versionInfo: full.versionInfo } + : null; folderAnnotateHistoryCache.set(resolvedFilePath, result); return result; } diff --git a/packages/server/annotate.ts b/packages/server/annotate.ts index a6d0f8d39..495358e2f 100644 --- a/packages/server/annotate.ts +++ b/packages/server/annotate.ts @@ -15,7 +15,7 @@ import { isRemoteSession, getServerHostname, startBunServerOnAvailablePort } fro import { getRepoInfo } from "./repo"; import type { Origin } from "@plannotator/shared/agents"; import { handleImage, handleUpload, handleServerReady, handleDraftSave, handleDraftLoad, handleDraftDelete, handleApiNotFound, handleFavicon, handleSaveNotes, readDraftGenerationFromBody, readDraftGenerationFromUrl } from "./shared-handlers"; -import { handleDoc, handleDocExists, handleFileBrowserFiles, handleObsidianVaults, handleObsidianFiles, handleObsidianDoc, resolveAllowedDocPath } from "./reference-handlers"; +import { handleDoc, handleDocExists, handleFileBrowserFiles, handleObsidianVaults, handleObsidianFiles, handleObsidianDoc, resolveAllowedDocPath, type FolderAnnotateHistory } from "./reference-handlers"; import { handleFileBrowserFilesStream } from "./reference-watch"; import { resolveUserPath, warmFileListCache } from "@plannotator/shared/resolve-file"; import { contentHash, deleteDraft } from "./draft"; @@ -187,11 +187,17 @@ export async function startAnnotateServer( // first time a folder file is opened via /api/doc (not eagerly for every // file in the folder) and memoized per resolved absolute path for the life // of this server — reopening the same file in this session never re-snapshots. - const folderAnnotateHistoryCache = new Map(); - function computeFolderAnnotateHistory(resolvedFilePath: string, content: string): AnnotateHistoryResult | null { + // The memo drops `diffCurrent` (it always equals the request's own content + // and the client never reads it off /api/doc) — only slug/previousPlan/ + // versionInfo are retained. + const folderAnnotateHistoryCache = new Map(); + function computeFolderAnnotateHistory(resolvedFilePath: string, content: string): FolderAnnotateHistory | null { const cached = folderAnnotateHistoryCache.get(resolvedFilePath); if (cached !== undefined) return cached; - const result = computeAnnotateHistory(annotateProjectName, resolvedFilePath, content); + const full = computeAnnotateHistory(annotateProjectName, resolvedFilePath, content); + const result: FolderAnnotateHistory | null = full + ? { slug: full.slug, previousPlan: full.previousPlan, versionInfo: full.versionInfo } + : null; folderAnnotateHistoryCache.set(resolvedFilePath, result); return result; } diff --git a/packages/server/reference-handlers.ts b/packages/server/reference-handlers.ts index 6d23ba6b7..b308d4598 100644 --- a/packages/server/reference-handlers.ts +++ b/packages/server/reference-handlers.ts @@ -41,6 +41,15 @@ import { import type { AnnotateHistoryResult } from "@plannotator/shared/annotate-history"; import { preloadFile } from "@pierre/diffs/ssr"; +/** + * Subset of AnnotateHistoryResult the folder /api/doc path actually needs. + * `diffCurrent` is omitted: it always equals the request's own `content` and + * the client never reads it off this response (the single-file /api/plan + * payload still returns the full AnnotateHistoryResult, `diffCurrent` + * included, for legacy shape parity — see annotate.ts). + */ +export type FolderAnnotateHistory = Omit; + // --- Route handlers --- /** @@ -61,14 +70,16 @@ export interface HandleDocOptions { * When set, /api/doc runs annotate's per-file version-history pipeline for * eligible markdown-branch documents (local file under an allowed root, * .md/.txt, not HTML, not a converted doc, under the annotatable size cap - * already enforced above) and merges `previousPlan`/`versionInfo`/ - * `diffCurrent` into the response — the same field names the single-file - * /api/plan payload uses. `compute` is expected to memoize per resolved - * path itself (the annotate server keys its cache by path in its own - * closure); this module only decides *whether* to call it. + * already enforced above) and merges `previousPlan`/`versionInfo` into the + * response — the same field names the single-file /api/plan payload uses + * (which additionally returns `diffCurrent`; the folder path omits it + * since it always equals the document's own content and the client never + * reads it). `compute` is expected to memoize per resolved path itself + * (the annotate server keys its cache by path in its own closure); this + * module only decides *whether* to call it. */ annotateHistory?: { - compute: (resolvedFilePath: string, content: string) => AnnotateHistoryResult | null; + compute: (resolvedFilePath: string, content: string) => FolderAnnotateHistory | null; }; } @@ -200,7 +211,6 @@ type DocOptionsResult = T & { sourceSave?: SourceSaveCapability; previousPlan?: string | null; versionInfo?: AnnotateHistoryResult["versionInfo"]; - diffCurrent?: string; }; function applyDocOptions>( @@ -234,7 +244,6 @@ function applyDocOptions>( if (history) { next.previousPlan = history.previousPlan; next.versionInfo = history.versionInfo; - next.diffCurrent = history.diffCurrent; } } if (typeof data.filepath !== "string") { diff --git a/packages/ui/hooks/useLinkedDoc.ts b/packages/ui/hooks/useLinkedDoc.ts index ee47b9a8e..0d77fe360 100644 --- a/packages/ui/hooks/useLinkedDoc.ts +++ b/packages/ui/hooks/useLinkedDoc.ts @@ -31,9 +31,6 @@ export interface LinkedDocLoadData { */ previousPlan?: string | null; versionInfo?: VersionInfo | null; - /** Present alongside previousPlan/versionInfo; not currently consumed by - * the client (mirrors the unused /api/plan field for shape parity). */ - diffCurrent?: string; } /** From 6d81d8b5916351a582f3287b0548f6360af35e55 Mon Sep 17 00:00:00 2001 From: Ben Newman <158209000+BenNewman100@users.noreply.github.com> Date: Tue, 21 Jul 2026 19:39:58 -0400 Subject: [PATCH 11/15] test(annotate): stop leaking history dirs; update diffCurrent expectations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The folder annotate history tests (Bun and Pi) minted a fresh project namespace per test but never cleaned up, leaving hundreds of directories under the real ~/.plannotator/history over repeated runs. Track every minted project and remove its history directory in afterAll — this also covers the stray non-directory artifact the "unwritable data dir" test deliberately plants inside its own project's history dir, since removing the project dir recursively takes it with it. Also update the two assertions that expected diffCurrent on the folder /api/doc response: that field is no longer propagated on the folder path (see the preceding diffCurrent-removal commit), so both now assert its absence instead. --- .../server/annotate-history.test.ts | 27 +++++++++++++--- packages/server/annotate.test.ts | 31 ++++++++++++++----- 2 files changed, 46 insertions(+), 12 deletions(-) diff --git a/apps/pi-extension/server/annotate-history.test.ts b/apps/pi-extension/server/annotate-history.test.ts index df152d7e1..d19280341 100644 --- a/apps/pi-extension/server/annotate-history.test.ts +++ b/apps/pi-extension/server/annotate-history.test.ts @@ -15,8 +15,8 @@ * approach as the Bun-side suite) so runs never collide. */ -import { afterEach, beforeEach, describe, expect, test } from "bun:test"; -import { mkdirSync, mkdtempSync, realpathSync, writeFileSync } from "node:fs"; +import { afterAll, afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { mkdirSync, mkdtempSync, realpathSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { startAnnotateServer } from "./serverAnnotate"; @@ -49,10 +49,25 @@ describe("pi annotate server: folder annotate history", () => { else process.env.PLANNOTATOR_ANNOTATE_HISTORY = savedHistoryFlag; }); + // Every minted name is tracked and its history directory removed in + // afterAll below — this suite must never leave residue in the real data + // dir (including the stray non-directory file the "unwritable data dir" + // test deliberately plants inside its own project's history dir; removing + // the project dir recursively takes that with it). + const mintedProjects: string[] = []; function uniqueProject(label: string): string { - return `_pi_annotate_history_test_${label}_${Date.now()}_${Math.random().toString(36).slice(2, 8)}`; + const project = `_pi_annotate_history_test_${label}_${Date.now()}_${Math.random().toString(36).slice(2, 8)}`; + mintedProjects.push(project); + return project; } + afterAll(() => { + const historyDir = join(getPlannotatorDataDir(), "history"); + for (const project of mintedProjects) { + rmSync(join(historyDir, project), { recursive: true, force: true }); + } + }); + test("first open mints one version; reopening in the same session is memoized (no re-snapshot even if the file changes on disk)", async () => { const folderPath = mkdtempSync(join(tmpdir(), "plannotator-pi-folder-history-first-open-")); const docPath = join(folderPath, "note.md"); @@ -74,12 +89,14 @@ describe("pi annotate server: folder annotate history", () => { markdown?: string; previousPlan?: string | null; versionInfo?: { version: number; totalVersions: number; project: string }; - diffCurrent?: string; }; expect(firstJson.markdown).toBe("V1\n"); expect(firstJson.previousPlan).toBeNull(); expect(firstJson.versionInfo).toEqual({ version: 1, totalVersions: 1, project }); - expect(firstJson.diffCurrent).toBe("V1\n"); + // diffCurrent is intentionally not propagated on the folder /api/doc + // path — it always equals the doc's own markdown and the client never + // reads it (unlike single-file /api/plan, which keeps it for shape parity). + expect("diffCurrent" in firstJson).toBe(false); // Change the file on disk between opens — a re-run of the pipeline // would mint version 2. Memoization must prevent that. diff --git a/packages/server/annotate.test.ts b/packages/server/annotate.test.ts index 5dd186246..57cc0b5b4 100644 --- a/packages/server/annotate.test.ts +++ b/packages/server/annotate.test.ts @@ -14,8 +14,8 @@ * process-global and cannot be unset). */ -import { afterEach, beforeEach, describe, expect, test } from "bun:test"; -import { existsSync, mkdirSync, mkdtempSync, readFileSync, realpathSync, symlinkSync, unlinkSync, writeFileSync } from "node:fs"; +import { afterAll, afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { existsSync, mkdirSync, mkdtempSync, readFileSync, realpathSync, rmSync, symlinkSync, unlinkSync, writeFileSync } from "node:fs"; import { tmpdir } from "os"; import { join } from "path"; import { startAnnotateServer } from "./annotate"; @@ -534,10 +534,25 @@ describe("annotate server: folder annotate history", () => { // Every test uses its own project namespace (history lives in the real // ~/.plannotator data dir, same as storage.test.ts) so runs never collide. + // Every minted name is tracked and its history directory removed in + // afterAll below — this suite must never leave residue in the real data + // dir (including the stray non-directory file the "unwritable data dir" + // test deliberately plants inside its own project's history dir; removing + // the project dir recursively takes that with it). + const mintedProjects: string[] = []; function uniqueProject(label: string): string { - return `_annotate_history_test_${label}_${Date.now()}_${Math.random().toString(36).slice(2, 8)}`; + const project = `_annotate_history_test_${label}_${Date.now()}_${Math.random().toString(36).slice(2, 8)}`; + mintedProjects.push(project); + return project; } + afterAll(() => { + const historyDir = join(getPlannotatorDataDir(), "history"); + for (const project of mintedProjects) { + rmSync(join(historyDir, project), { recursive: true, force: true }); + } + }); + test("first open mints one version; reopening in the same session is memoized (no re-snapshot even if the file changes on disk)", async () => { const folderPath = mkdtempSync(join(tmpdir(), "plannotator-folder-history-first-open-")); const docPath = join(folderPath, "note.md"); @@ -559,12 +574,14 @@ describe("annotate server: folder annotate history", () => { markdown?: string; previousPlan?: string | null; versionInfo?: { version: number; totalVersions: number; project: string }; - diffCurrent?: string; }; expect(firstJson.markdown).toBe("V1\n"); expect(firstJson.previousPlan).toBeNull(); expect(firstJson.versionInfo).toEqual({ version: 1, totalVersions: 1, project }); - expect(firstJson.diffCurrent).toBe("V1\n"); + // diffCurrent is intentionally not propagated on the folder /api/doc + // path — it always equals the doc's own markdown and the client never + // reads it (unlike single-file /api/plan, which keeps it for shape parity). + expect("diffCurrent" in firstJson).toBe(false); // Change the file on disk between opens — a re-run of the pipeline // would mint version 2. Memoization must prevent that. @@ -698,11 +715,11 @@ describe("annotate server: folder annotate history", () => { const json = await response.json() as { previousPlan?: string | null; versionInfo?: { version: number; totalVersions: number; project: string }; - diffCurrent?: string; }; expect(json.previousPlan).toBeNull(); expect(json.versionInfo).toEqual({ version: 1, totalVersions: 1, project }); - expect(json.diffCurrent).toBe("Fresh\n"); + // diffCurrent is intentionally not propagated on the folder /api/doc path. + expect("diffCurrent" in json).toBe(false); } finally { server.stop(); } From 10b231d536f7e8377a400633d85333571e9dc07b Mon Sep 17 00:00:00 2001 From: Ben Newman <158209000+BenNewman100@users.noreply.github.com> Date: Tue, 21 Jul 2026 19:40:18 -0400 Subject: [PATCH 12/15] fix(ui): remember per-document diff-base selection across navigation usePlanDiff reset diffBasePlan/diffBaseVersion to the newly-provided document's defaults on every docKey change. That discarded a manually selected base version when navigating away from a document and back (e.g. root -> linked doc -> root), regressing behavior upstream relied on keeping (nothing reset the selection before this seam existed). Track each docKey's selection in a ref-held Map (keyed by docKey, including null for the root document) and restore it on return instead of re-seeding defaults; a key visited for the first time still seeds from its own initialPreviousPlan/versionInfo exactly as before, and selections never leak between distinct keys. Adds two DOM-gated tests: restoring a manual selection after a detour to another document, and confirming distinct docKeys don't leak into each other. --- packages/ui/hooks/usePlanDiff.test.tsx | 112 +++++++++++++++++++++++++ packages/ui/hooks/usePlanDiff.ts | 64 ++++++++++---- 2 files changed, 161 insertions(+), 15 deletions(-) diff --git a/packages/ui/hooks/usePlanDiff.test.tsx b/packages/ui/hooks/usePlanDiff.test.tsx index 0b4856143..9a85ade53 100644 --- a/packages/ui/hooks/usePlanDiff.test.tsx +++ b/packages/ui/hooks/usePlanDiff.test.tsx @@ -150,6 +150,118 @@ describe('usePlanDiff — per-document docKey reset', () => { }); }); +describe('usePlanDiff — per-docKey base-selection memory', () => { + test.skipIf(!hasDom)('a manually-selected base version survives a detour to another document and back', async () => { + const rootVersionInfo: VersionInfo = { version: 3, totalVersions: 3, project: 'demo' }; + const docAVersionInfo: VersionInfo = { version: 5, totalVersions: 5, project: 'demo' }; + const fetchers: PlanDiffFetchers = { + fetchVersion: async (version) => ({ plan: `fetched plan v${version}`, version }), + }; + + const session = await mountHolder({ + currentPlan: 'root current text', + initialPreviousPlan: 'root v2 text', + versionInfo: rootVersionInfo, + docKey: null, + fetchers, + }); + + // Sanity: the auto-selected default base is version - 1 (v2), not v1. + expect(session.current().diffBaseVersion).toBe(2); + expect(session.current().diffBasePlan).toBe('root v2 text'); + + // User manually picks an earlier, non-default version via the injected fetcher. + await act(async () => { + await session.current().selectBaseVersion(1); + }); + expect(session.current().diffBaseVersion).toBe(1); + expect(session.current().diffBasePlan).toBe('fetched plan v1'); + + // Detour: navigate to a different document (e.g. a linked doc opened + // from root). Its own default base applies — nothing carries over. + await session.update({ + currentPlan: 'docA current text', + initialPreviousPlan: 'docA previous text', + versionInfo: docAVersionInfo, + docKey: 'docA', + fetchers, + }); + expect(session.current().diffBaseVersion).toBe(4); + expect(session.current().diffBasePlan).toBe('docA previous text'); + + // Navigate back to root (docKey null) with the same original props. + await session.update({ + currentPlan: 'root current text', + initialPreviousPlan: 'root v2 text', + versionInfo: rootVersionInfo, + docKey: null, + fetchers, + }); + + // The manual selection from before the detour is restored — NOT the + // auto-computed default (v2) that a plain reset would re-seed. + expect(session.current().diffBaseVersion).toBe(1); + expect(session.current().diffBasePlan).toBe('fetched plan v1'); + }); + + test.skipIf(!hasDom)('distinct docKeys keep independent base selections — no leakage between them', async () => { + const versionInfoA: VersionInfo = { version: 3, totalVersions: 3, project: 'demo' }; + const versionInfoB: VersionInfo = { version: 4, totalVersions: 4, project: 'demo' }; + const fetchers: PlanDiffFetchers = { + fetchVersion: async (version) => ({ plan: `fetched plan v${version}`, version }), + }; + + const session = await mountHolder({ + currentPlan: 'docA current', + initialPreviousPlan: 'docA previous', + versionInfo: versionInfoA, + docKey: 'docA', + fetchers, + }); + await act(async () => { + await session.current().selectBaseVersion(1); + }); + expect(session.current().diffBaseVersion).toBe(1); + expect(session.current().diffBasePlan).toBe('fetched plan v1'); + + await session.update({ + currentPlan: 'docB current', + initialPreviousPlan: 'docB previous', + versionInfo: versionInfoB, + docKey: 'docB', + fetchers, + }); + await act(async () => { + await session.current().selectBaseVersion(2); + }); + expect(session.current().diffBaseVersion).toBe(2); + expect(session.current().diffBasePlan).toBe('fetched plan v2'); + + // Back to docA: its own selection (v1) — not docB's (v2). + await session.update({ + currentPlan: 'docA current', + initialPreviousPlan: 'docA previous', + versionInfo: versionInfoA, + docKey: 'docA', + fetchers, + }); + expect(session.current().diffBaseVersion).toBe(1); + expect(session.current().diffBasePlan).toBe('fetched plan v1'); + + // Back to docB: its own selection (v2) — confirms docA's visit above + // didn't clobber it either. + await session.update({ + currentPlan: 'docB current', + initialPreviousPlan: 'docB previous', + versionInfo: versionInfoB, + docKey: 'docB', + fetchers, + }); + expect(session.current().diffBaseVersion).toBe(2); + expect(session.current().diffBasePlan).toBe('fetched plan v2'); + }); +}); + describe('usePlanDiff — baseline survives a live-reload-style text update', () => { test.skipIf(!hasDom)('a currentPlan change with a stable docKey recomputes the diff without losing diffBasePlan', async () => { const versionInfoA: VersionInfo = { version: 2, totalVersions: 2, project: 'demo' }; diff --git a/packages/ui/hooks/usePlanDiff.ts b/packages/ui/hooks/usePlanDiff.ts index 883650008..a57d6fceb 100644 --- a/packages/ui/hooks/usePlanDiff.ts +++ b/packages/ui/hooks/usePlanDiff.ts @@ -93,13 +93,16 @@ export function usePlanDiff( /** * Identity of the document `initialPreviousPlan`/`versionInfo` belong to * (e.g. a folder/linked document's filepath). When this changes between - * renders, the diff-base state below resets to the newly-provided - * `initialPreviousPlan`/`versionInfo` instead of keeping whatever the - * previously active document had selected — otherwise switching documents - * would silently keep diffing the new document's text against the old - * document's base plan. Omit (or keep it referentially stable, as the root - * document does for the life of a session) to preserve exactly today's - * one-time-hydration behavior via the two sync effects below. + * renders, the diff-base state below switches to that document's own + * remembered base selection (if this document was visited earlier in the + * session — see baseSelectionsByDocKeyRef below) or, the first time a + * document is seen, seeds from its `initialPreviousPlan`/`versionInfo` — + * either way, never keeping whatever the previously active document had + * selected, since that would silently keep diffing the new document's text + * against the old document's base plan. Omit (or keep it referentially + * stable, as the root document does for the life of a session) to preserve + * exactly today's one-time-hydration behavior via the two sync effects + * below. */ docKey?: string | null ): UsePlanDiffReturn { @@ -130,19 +133,50 @@ export function usePlanDiff( } }, [versionInfo]); - // Reset diff-base state whenever the active document identity changes - // (e.g. folder/linked-doc navigation). docKey is undefined/stable for a - // session with no per-doc identity (the root document), so this never - // fires there and the sync effects above keep owning its one-time + // Remember each document's diff-base selection across navigation, keyed by + // docKey (including `null`/`undefined`) — so returning to a previously + // visited document (e.g. the root doc after a linked-doc detour) restores + // whatever base version the user had picked there instead of re-seeding + // defaults. A plain ref (not state) since writing it must never itself + // trigger a render — it's only read/written from inside the docKey-change + // effect below. Never shared across keys: each key's entry is only ever + // populated from that same key's own state at the moment it stops being + // active. + const baseSelectionsByDocKeyRef = useRef( + new Map< + string | null | undefined, + { diffBasePlan: string | null; diffBaseVersion: number | null } + >() + ); + + // Reset (or restore) diff-base state whenever the active document identity + // changes (e.g. folder/linked-doc navigation). docKey is undefined/stable + // for a session with no per-doc identity (the root document), so this + // never fires there and the sync effects above keep owning its one-time // hydration — byte-identical to before this seam existed. const prevDocKeyRef = useRef(docKey); useEffect(() => { if (prevDocKeyRef.current === docKey) return; + + // Persist the outgoing document's current selection before switching, so + // coming back to it later (even after other documents were visited in + // between) restores it rather than re-seeding defaults. + baseSelectionsByDocKeyRef.current.set(prevDocKeyRef.current, { + diffBasePlan, + diffBaseVersion, + }); prevDocKeyRef.current = docKey; - setDiffBasePlan(initialPreviousPlan); - setDiffBaseVersion( - versionInfo && versionInfo.version > 1 ? versionInfo.version - 1 : null - ); + + const remembered = baseSelectionsByDocKeyRef.current.get(docKey); + if (remembered) { + setDiffBasePlan(remembered.diffBasePlan); + setDiffBaseVersion(remembered.diffBaseVersion); + } else { + setDiffBasePlan(initialPreviousPlan); + setDiffBaseVersion( + versionInfo && versionInfo.version > 1 ? versionInfo.version - 1 : null + ); + } setVersions([]); setIsLoadingVersions(false); setIsSelectingVersion(false); From d90a8cfee732b2468d3fc7475a529322e738c53f Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Wed, 22 Jul 2026 08:26:07 -0700 Subject: [PATCH 13/15] fix(annotate): match folder history eligibility to the single-file plain-text set MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The folder /api/doc history gate was a hardcoded /\.(md|txt)$/i in both runtimes, so any other annotatable plain-text file (.mdx, .yaml, .json, .toml, ...) opened via a folder session silently skipped snapshotting — breaking the cross-mode continuity this feature advertises (a .yaml with an existing single-file version thread showed no diff when opened via its folder). Reuse the canonical predicate instead: isAnnotatableTextPath (ANNOTATABLE_TEXT_REGEX in @plannotator/core/annotatable), the exact set the single-file pipeline snapshots. HTML stays deferred and .env stays excluded, both by that same definition. Tests extended in both runtimes: .mdx mints on first open, .yaml single-file history serves as the folder baseline, .env mints nothing, .html unchanged. --- .../server/annotate-history.test.ts | 102 ++++++++++++++++++ apps/pi-extension/server/reference.ts | 25 ++--- packages/server/annotate.test.ts | 102 ++++++++++++++++++ packages/server/reference-handlers.ts | 25 ++--- packages/ui/hooks/useLinkedDoc.ts | 6 +- 5 files changed, 234 insertions(+), 26 deletions(-) diff --git a/apps/pi-extension/server/annotate-history.test.ts b/apps/pi-extension/server/annotate-history.test.ts index d19280341..a66122c32 100644 --- a/apps/pi-extension/server/annotate-history.test.ts +++ b/apps/pi-extension/server/annotate-history.test.ts @@ -231,6 +231,108 @@ describe("pi annotate server: folder annotate history", () => { } }); + test("eligibility matches the single-file plain-text set: .mdx mints a snapshot on first open", async () => { + const folderPath = mkdtempSync(join(tmpdir(), "plannotator-pi-folder-history-mdx-")); + const docPath = join(folderPath, "note.mdx"); + writeFileSync(docPath, "MDX content\n", "utf-8"); + const project = uniqueProject("mdx"); + + const server = await startAnnotateServer({ + markdown: "", + filePath: folderPath, + folderPath, + mode: "annotate-folder", + htmlContent: "", + project, + }); + + try { + const response = await fetch(`${server.url}/api/doc?path=${encodeURIComponent(docPath)}`); + const json = (await response.json()) as { + previousPlan?: string | null; + versionInfo?: { version: number; totalVersions: number; project: string }; + }; + expect(json.previousPlan).toBeNull(); + expect(json.versionInfo).toEqual({ version: 1, totalVersions: 1, project }); + } finally { + server.stop(); + } + }); + + test("cross-mode continuity for config formats: a .yaml with single-file history diffs when opened via its folder", async () => { + const folderPath = mkdtempSync(join(tmpdir(), "plannotator-pi-folder-history-yaml-")); + const docPath = join(folderPath, "config.yaml"); + const project = uniqueProject("yaml"); + + // Single-file session saves "a: 1" as version 1 for this exact path. + const seedServer = await startAnnotateServer({ + markdown: "a: 1\n", + filePath: docPath, + htmlContent: "", + mode: "annotate", + project, + }); + seedServer.stop(); + + // The folder session reads different content off disk, so it mints version 2. + writeFileSync(docPath, "a: 2\n", "utf-8"); + + const server = await startAnnotateServer({ + markdown: "", + filePath: folderPath, + folderPath, + mode: "annotate-folder", + htmlContent: "", + project, + }); + + try { + // `doc=1` mirrors the file browser: it forces annotatable plain-text + // rendering for extensions that overlap CODE_FILE_REGEX (.yaml, .json…). + const response = await fetch(`${server.url}/api/doc?path=${encodeURIComponent(docPath)}&doc=1`); + const json = (await response.json()) as { + previousPlan?: string | null; + versionInfo?: { version: number; totalVersions: number; project: string }; + }; + expect(json.previousPlan).toBe("a: 1\n"); + expect(json.versionInfo).toEqual({ version: 2, totalVersions: 2, project }); + } finally { + server.stop(); + } + }); + + test(".env stays ineligible: no snapshot is minted even though .env.example would be", async () => { + const folderPath = mkdtempSync(join(tmpdir(), "plannotator-pi-folder-history-env-")); + const docPath = join(folderPath, ".env"); + writeFileSync(docPath, "SECRET=1\n", "utf-8"); + const project = uniqueProject("env"); + + const server = await startAnnotateServer({ + markdown: "", + filePath: folderPath, + folderPath, + mode: "annotate-folder", + htmlContent: "", + project, + }); + + try { + const response = await fetch(`${server.url}/api/doc?path=${encodeURIComponent(docPath)}&doc=1`); + const json = (await response.json()) as Record; + // Whatever shape /api/doc answers with (.env is not annotatable, so it + // is never served as a document), no history fields may appear and no + // snapshot may be written. + expect("previousPlan" in json).toBe(false); + expect("versionInfo" in json).toBe(false); + + const versions = await fetch(`${server.url}/api/plan/versions?path=${encodeURIComponent(docPath)}`); + const versionsJson = (await versions.json()) as { slug: string | null; versions: unknown[] }; + expect(versionsJson).toEqual({ project, slug: null, versions: [] }); + } finally { + server.stop(); + } + }); + test("an unwritable history directory degrades to a plain render, no error propagates", async () => { const folderPath = mkdtempSync(join(tmpdir(), "plannotator-pi-folder-history-unwritable-")); const docPath = join(folderPath, "note.md"); diff --git a/apps/pi-extension/server/reference.ts b/apps/pi-extension/server/reference.ts index 77f2b9caa..2d9d021bf 100644 --- a/apps/pi-extension/server/reference.ts +++ b/apps/pi-extension/server/reference.ts @@ -65,13 +65,12 @@ export type FolderAnnotateHistory = Omit; type Res = ServerResponse; -/** - * Extensions eligible for annotate's per-file version history when served - * through /api/doc (folder annotate mode). Deliberately narrower than the - * full annotatable-text set: markdown-branch documents only, matching the - * gate the folder annotate design settled on. Mirrors packages/server/reference-handlers.ts. - */ -const ANNOTATE_HISTORY_ELIGIBLE_REGEX = /\.(md|txt)$/i; +// History eligibility for folder /api/doc documents is `isAnnotatableTextPath` +// (ANNOTATABLE_TEXT_REGEX in @plannotator/core/annotatable) — the exact set the +// single-file pipeline snapshots (.md/.mdx/.txt plus the plain-text config +// formats; no HTML, no .env). Reusing the canonical predicate keeps cross-mode +// slug continuity: a .yaml with single-file history must diff when opened via +// its folder too. Mirrors packages/server/reference-handlers.ts. export interface HandleDocOptions { rewriteHtml?: (html: string, filepath: string) => string; @@ -82,8 +81,9 @@ export interface HandleDocOptions { /** * When set, /api/doc runs annotate's per-file version-history pipeline for * eligible markdown-branch documents (local file under an allowed root, - * .md/.txt, not HTML, not a converted doc, under the annotatable size cap - * already enforced above) and merges `previousPlan`/`versionInfo` into the + * any annotatable plain-text extension per `isAnnotatableTextPath`, not + * HTML, not a converted doc, under the annotatable size cap already + * enforced above) and merges `previousPlan`/`versionInfo` into the * response — the same field names the single-file /api/plan payload uses * (which additionally returns `diffCurrent`; the folder path omits it * since it always equals the document's own content and the client never @@ -242,8 +242,9 @@ function applyDocOptions>( } // Annotate version history (folder mode only — see HandleDocOptions.annotateHistory). // Independent of the sourceSave branching below: only markdown-branch - // documents (not HTML, not converted) with a .md/.txt extension are - // eligible. The 2MB annotatable-file size cap is already enforced by the + // documents (not HTML, not converted) with an annotatable plain-text + // extension are eligible — the same set the single-file pipeline + // snapshots. The 2MB annotatable-file size cap is already enforced by the // caller before any of these responses are built, so no separate check is // needed here. Mirrors packages/server/reference-handlers.ts. if ( @@ -252,7 +253,7 @@ function applyDocOptions>( data.renderAs === "markdown" && data.isConverted !== true && typeof data.markdown === "string" && - ANNOTATE_HISTORY_ELIGIBLE_REGEX.test(data.filepath) + isAnnotatableTextPath(data.filepath) ) { const history = options.annotateHistory.compute(data.filepath, data.markdown); if (history) { diff --git a/packages/server/annotate.test.ts b/packages/server/annotate.test.ts index 57cc0b5b4..ac4e877ba 100644 --- a/packages/server/annotate.test.ts +++ b/packages/server/annotate.test.ts @@ -790,6 +790,108 @@ describe("annotate server: folder annotate history", () => { } }); + test("eligibility matches the single-file plain-text set: .mdx mints a snapshot on first open", async () => { + const folderPath = mkdtempSync(join(tmpdir(), "plannotator-folder-history-mdx-")); + const docPath = join(folderPath, "note.mdx"); + writeFileSync(docPath, "MDX content\n", "utf-8"); + const project = uniqueProject("mdx"); + + const server = await startAnnotateServer({ + markdown: "", + filePath: folderPath, + folderPath, + mode: "annotate-folder", + htmlContent: MINIMAL_HTML, + project, + }); + + try { + const response = await fetch(`${server.url}/api/doc?path=${encodeURIComponent(docPath)}`); + const json = await response.json() as { + previousPlan?: string | null; + versionInfo?: { version: number; totalVersions: number; project: string }; + }; + expect(json.previousPlan).toBeNull(); + expect(json.versionInfo).toEqual({ version: 1, totalVersions: 1, project }); + } finally { + server.stop(); + } + }); + + test("cross-mode continuity for config formats: a .yaml with single-file history diffs when opened via its folder", async () => { + const folderPath = mkdtempSync(join(tmpdir(), "plannotator-folder-history-yaml-")); + const docPath = join(folderPath, "config.yaml"); + const project = uniqueProject("yaml"); + + // Single-file session saves "a: 1" as version 1 for this exact path. + const seedServer = await startAnnotateServer({ + markdown: "a: 1\n", + filePath: docPath, + htmlContent: MINIMAL_HTML, + mode: "annotate", + project, + }); + seedServer.stop(); + + // The folder session reads different content off disk, so it mints version 2. + writeFileSync(docPath, "a: 2\n", "utf-8"); + + const server = await startAnnotateServer({ + markdown: "", + filePath: folderPath, + folderPath, + mode: "annotate-folder", + htmlContent: MINIMAL_HTML, + project, + }); + + try { + // `doc=1` mirrors the file browser: it forces annotatable plain-text + // rendering for extensions that overlap CODE_FILE_REGEX (.yaml, .json…). + const response = await fetch(`${server.url}/api/doc?path=${encodeURIComponent(docPath)}&doc=1`); + const json = await response.json() as { + previousPlan?: string | null; + versionInfo?: { version: number; totalVersions: number; project: string }; + }; + expect(json.previousPlan).toBe("a: 1\n"); + expect(json.versionInfo).toEqual({ version: 2, totalVersions: 2, project }); + } finally { + server.stop(); + } + }); + + test(".env stays ineligible: no snapshot is minted even though .env.example would be", async () => { + const folderPath = mkdtempSync(join(tmpdir(), "plannotator-folder-history-env-")); + const docPath = join(folderPath, ".env"); + writeFileSync(docPath, "SECRET=1\n", "utf-8"); + const project = uniqueProject("env"); + + const server = await startAnnotateServer({ + markdown: "", + filePath: folderPath, + folderPath, + mode: "annotate-folder", + htmlContent: MINIMAL_HTML, + project, + }); + + try { + const response = await fetch(`${server.url}/api/doc?path=${encodeURIComponent(docPath)}&doc=1`); + const json = await response.json() as Record; + // Whatever shape /api/doc answers with (.env is not annotatable, so it + // is never served as a document), no history fields may appear and no + // snapshot may be written. + expect("previousPlan" in json).toBe(false); + expect("versionInfo" in json).toBe(false); + + const versions = await fetch(`${server.url}/api/plan/versions?path=${encodeURIComponent(docPath)}`); + const versionsJson = await versions.json() as { slug: string | null; versions: unknown[] }; + expect(versionsJson).toEqual({ project, slug: null, versions: [] }); + } finally { + server.stop(); + } + }); + test("an unwritable history directory degrades to a plain render, no error propagates", async () => { const folderPath = mkdtempSync(join(tmpdir(), "plannotator-folder-history-unwritable-")); const docPath = join(folderPath, "note.md"); diff --git a/packages/server/reference-handlers.ts b/packages/server/reference-handlers.ts index b308d4598..b9bef7779 100644 --- a/packages/server/reference-handlers.ts +++ b/packages/server/reference-handlers.ts @@ -52,13 +52,12 @@ export type FolderAnnotateHistory = Omit; // --- Route handlers --- -/** - * Extensions eligible for annotate's per-file version history when served - * through /api/doc (folder annotate mode). Deliberately narrower than the - * full annotatable-text set: markdown-branch documents only, matching the - * gate the folder annotate design settled on. - */ -const ANNOTATE_HISTORY_ELIGIBLE_REGEX = /\.(md|txt)$/i; +// History eligibility for folder /api/doc documents is `isAnnotatableTextPath` +// (ANNOTATABLE_TEXT_REGEX in @plannotator/core/annotatable) — the exact set the +// single-file pipeline snapshots (.md/.mdx/.txt plus the plain-text config +// formats; no HTML, no .env). Reusing the canonical predicate keeps cross-mode +// slug continuity: a .yaml with single-file history must diff when opened via +// its folder too. export interface HandleDocOptions { rewriteHtml?: (html: string, filepath: string) => string; @@ -69,8 +68,9 @@ export interface HandleDocOptions { /** * When set, /api/doc runs annotate's per-file version-history pipeline for * eligible markdown-branch documents (local file under an allowed root, - * .md/.txt, not HTML, not a converted doc, under the annotatable size cap - * already enforced above) and merges `previousPlan`/`versionInfo` into the + * any annotatable plain-text extension per `isAnnotatableTextPath`, not + * HTML, not a converted doc, under the annotatable size cap already + * enforced above) and merges `previousPlan`/`versionInfo` into the * response — the same field names the single-file /api/plan payload uses * (which additionally returns `diffCurrent`; the folder path omits it * since it always equals the document's own content and the client never @@ -228,8 +228,9 @@ function applyDocOptions>( } // Annotate version history (folder mode only — see HandleDocOptions.annotateHistory). // Independent of the sourceSave branching below: only markdown-branch - // documents (not HTML, not converted) with a .md/.txt extension are - // eligible. The 2MB annotatable-file size cap is already enforced by the + // documents (not HTML, not converted) with an annotatable plain-text + // extension are eligible — the same set the single-file pipeline + // snapshots. The 2MB annotatable-file size cap is already enforced by the // caller before any of these responses are built, so no separate check is // needed here. if ( @@ -238,7 +239,7 @@ function applyDocOptions>( data.renderAs === "markdown" && data.isConverted !== true && typeof data.markdown === "string" && - ANNOTATE_HISTORY_ELIGIBLE_REGEX.test(data.filepath) + isAnnotatableTextPath(data.filepath) ) { const history = options.annotateHistory.compute(data.filepath, data.markdown); if (history) { diff --git a/packages/ui/hooks/useLinkedDoc.ts b/packages/ui/hooks/useLinkedDoc.ts index 0d77fe360..fd3dd62f3 100644 --- a/packages/ui/hooks/useLinkedDoc.ts +++ b/packages/ui/hooks/useLinkedDoc.ts @@ -24,8 +24,10 @@ export interface LinkedDocLoadData { /** * Per-file version-diff baseline (annotate folder sessions only) — the same * field names/shapes /api/plan already returns for single-file sessions. - * /api/doc only populates these for eligible folder files (local .md/.txt, - * markdown-branch, not converted/HTML); every other document — including + * /api/doc only populates these for eligible folder files (local annotatable + * plain-text files — .md/.mdx/.txt plus the config set, per + * `isAnnotatableTextPath` — markdown-branch, not converted/HTML); every other + * document — including * plain linked docs outside folder mode — omits them, which is what keeps * the diff badge/version browser from appearing for those. */ From 700918a71dd9b6bea681ef4a2e2963f17cf897f5 Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Wed, 22 Jul 2026 08:26:25 -0700 Subject: [PATCH 14/15] feat(ui): label the folder diff badge with its baseline MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The in-file version-diff badge in annotate/folder sessions shows +N/-M against the file's last-reviewed snapshot, while the git badges in the file tree count uncommitted-vs-HEAD — same numbers, different baselines. Give the badge an optional baseline suffix and tooltip override (PlanDiffBadge baselineLabel/baselineTooltip, threaded through DocBadges, Viewer, and StickyHeaderLane) and have annotate mode pass 'since last review' / 'Changes since you last reviewed this file'. Plan review passes nothing and renders byte-identically to before. DOM tests cover both the labeled and the unchanged default rendering. --- .github/workflows/test.yml | 1 + packages/editor/App.tsx | 4 +++ packages/ui/components/DocBadges.test.tsx | 30 +++++++++++++++++++ packages/ui/components/DocBadges.tsx | 9 ++++++ packages/ui/components/StickyHeaderLane.tsx | 7 +++++ packages/ui/components/Viewer.tsx | 8 +++++ .../ui/components/plan-diff/PlanDiffBadge.tsx | 23 +++++++++++++- 7 files changed, 81 insertions(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 2e2732ee8..aeef20e94 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -65,6 +65,7 @@ jobs: packages/ui/hooks/useFileBrowser.seam.test.tsx packages/ui/hooks/usePlanDiff.test.tsx packages/ui/hooks/useLinkedDoc.test.tsx + packages/ui/components/DocBadges.test.tsx pi-extension-ai-runtime-windows: # Exercises the Pi extension's Node/jiti server mirror on Windows with an diff --git a/packages/editor/App.tsx b/packages/editor/App.tsx index bac12f223..dc5dd8a71 100644 --- a/packages/editor/App.tsx +++ b/packages/editor/App.tsx @@ -4191,6 +4191,8 @@ const App: React.FC = () => { isPlanDiffActive={isPlanDiffActive} hasPreviousVersion={planDiff.hasPreviousVersion} onPlanDiffToggle={() => setIsPlanDiffActive(!isPlanDiffActive)} + planDiffBaselineLabel={annotateMode ? 'since last review' : undefined} + planDiffBaselineTooltip={annotateMode ? 'Changes since you last reviewed this file' : undefined} archiveInfo={archive.currentInfo} maxWidth={annotateReaderMaxWidth} remountToken={viewerContentKey} @@ -4433,6 +4435,8 @@ const App: React.FC = () => { isPlanDiffActive={isPlanDiffActive} onPlanDiffToggle={() => setIsPlanDiffActive(!isPlanDiffActive)} hasPreviousVersion={planDiff.hasPreviousVersion} + planDiffBaselineLabel={annotateMode ? 'since last review' : undefined} + planDiffBaselineTooltip={annotateMode ? 'Changes since you last reviewed this file' : undefined} showDemoBadge={!isApiMode && !isLoadingShared && !isSharedSession} maxWidth={annotateReaderMaxWidth} onOpenLinkedDoc={handleOpenLinkedDoc} diff --git a/packages/ui/components/DocBadges.test.tsx b/packages/ui/components/DocBadges.test.tsx index 4ff9ddc30..614f6e5a4 100644 --- a/packages/ui/components/DocBadges.test.tsx +++ b/packages/ui/components/DocBadges.test.tsx @@ -130,6 +130,36 @@ describe('DocBadges open-in placement', () => { expect(host?.querySelector('button[aria-label="Open in Finder"]')).toBeNull(); }); + test.skipIf(!hasDom)('labels the diff badge with its baseline in annotate/folder sessions', async () => { + await renderBadges({ + planDiffStats: { additions: 3, deletions: 1, modifications: 0 }, + hasPreviousVersion: true, + onPlanDiffToggle: () => {}, + planDiffBaselineLabel: 'since last review', + planDiffBaselineTooltip: 'Changes since you last reviewed this file', + }); + + const badge = Array.from(host?.querySelectorAll('button') ?? []) + .find((button) => button.textContent?.includes('+3')); + if (!badge) throw new Error('Expected the diff badge to render'); + expect(badge.textContent).toContain('since last review'); + expect(badge.title).toBe('Changes since you last reviewed this file'); + }); + + test.skipIf(!hasDom)('plan review diff badge renders exactly as before when no baseline label is passed', async () => { + await renderBadges({ + planDiffStats: { additions: 3, deletions: 1, modifications: 0 }, + hasPreviousVersion: true, + onPlanDiffToggle: () => {}, + }); + + const badge = Array.from(host?.querySelectorAll('button') ?? []) + .find((button) => button.textContent?.includes('+3')); + if (!badge) throw new Error('Expected the diff badge to render'); + expect(badge.textContent).toBe('+3/-1'); + expect(badge.title).toBe('Show what changed from previous version'); + }); + test.skipIf(!hasDom)('keeps folder-file selectors after the active filename', async () => { await renderBadges({ linkedDocInfo: { diff --git a/packages/ui/components/DocBadges.tsx b/packages/ui/components/DocBadges.tsx index 42871c5ce..79c25c01d 100644 --- a/packages/ui/components/DocBadges.tsx +++ b/packages/ui/components/DocBadges.tsx @@ -35,6 +35,11 @@ export interface DocBadgesProps { isPlanDiffActive?: boolean; hasPreviousVersion?: boolean; onPlanDiffToggle?: () => void; + /** Baseline suffix + tooltip for the plan-diff badge — annotate/folder + * sessions pass "since last review" so the badge's numbers read against + * the right baseline. See PlanDiffBadge. Plan review passes nothing. */ + planDiffBaselineLabel?: string; + planDiffBaselineTooltip?: string; showDemoBadge?: boolean; archiveInfo?: { status: 'approved' | 'denied' | 'unknown'; timestamp: string; title: string } | null; linkedDocInfo?: LinkedDocBadgeInfo | null; @@ -54,6 +59,8 @@ export const DocBadges: React.FC = ({ isPlanDiffActive, hasPreviousVersion, onPlanDiffToggle, + planDiffBaselineLabel, + planDiffBaselineTooltip, showDemoBadge, archiveInfo, linkedDocInfo, @@ -136,6 +143,8 @@ export const DocBadges: React.FC = ({ isActive={isPlanDiffActive ?? false} onToggle={onPlanDiffToggle} hasPreviousVersion={hasPreviousVersion ?? false} + baselineLabel={planDiffBaselineLabel} + baselineTooltip={planDiffBaselineTooltip} /> )} diff --git a/packages/ui/components/StickyHeaderLane.tsx b/packages/ui/components/StickyHeaderLane.tsx index 28e297710..a1986cc79 100644 --- a/packages/ui/components/StickyHeaderLane.tsx +++ b/packages/ui/components/StickyHeaderLane.tsx @@ -69,6 +69,9 @@ interface StickyHeaderLaneProps { isPlanDiffActive?: boolean; hasPreviousVersion?: boolean; onPlanDiffToggle?: () => void; + /** Baseline suffix + tooltip for the plan-diff badge (see DocBadges). */ + planDiffBaselineLabel?: string; + planDiffBaselineTooltip?: string; archiveInfo?: { status: 'approved' | 'denied' | 'unknown'; timestamp: string; title: string } | null; // Layout @@ -94,6 +97,8 @@ export const StickyHeaderLane: React.FC = ({ isPlanDiffActive, hasPreviousVersion, onPlanDiffToggle, + planDiffBaselineLabel, + planDiffBaselineTooltip, archiveInfo, maxWidth, remountToken, @@ -251,6 +256,8 @@ export const StickyHeaderLane: React.FC = ({ isPlanDiffActive={isPlanDiffActive} hasPreviousVersion={hasPreviousVersion} onPlanDiffToggle={onPlanDiffToggle} + planDiffBaselineLabel={planDiffBaselineLabel} + planDiffBaselineTooltip={planDiffBaselineTooltip} archiveInfo={archiveInfo} /> diff --git a/packages/ui/components/Viewer.tsx b/packages/ui/components/Viewer.tsx index 089442310..615228ce9 100644 --- a/packages/ui/components/Viewer.tsx +++ b/packages/ui/components/Viewer.tsx @@ -82,6 +82,10 @@ interface ViewerProps { isPlanDiffActive?: boolean; onPlanDiffToggle?: () => void; hasPreviousVersion?: boolean; + /** Baseline suffix + tooltip for the plan-diff badge (see DocBadges) — + * annotate/folder sessions pass "since last review"; plan review omits. */ + planDiffBaselineLabel?: string; + planDiffBaselineTooltip?: string; /** Show amber "Demo" badge (portal mode, no shared content loaded) */ showDemoBadge?: boolean; /** Max width in px for the plan card; null removes the cap entirely. */ @@ -181,6 +185,8 @@ export const Viewer = forwardRef(({ isPlanDiffActive, onPlanDiffToggle, hasPreviousVersion, + planDiffBaselineLabel, + planDiffBaselineTooltip, showDemoBadge, maxWidth, onOpenLinkedDoc, @@ -587,6 +593,8 @@ export const Viewer = forwardRef(({ isPlanDiffActive={isPlanDiffActive} hasPreviousVersion={hasPreviousVersion} onPlanDiffToggle={onPlanDiffToggle} + planDiffBaselineLabel={planDiffBaselineLabel} + planDiffBaselineTooltip={planDiffBaselineTooltip} showDemoBadge={showDemoBadge} archiveInfo={archiveInfo} linkedDocInfo={linkedDocInfo} diff --git a/packages/ui/components/plan-diff/PlanDiffBadge.tsx b/packages/ui/components/plan-diff/PlanDiffBadge.tsx index b6e0f4025..055470328 100644 --- a/packages/ui/components/plan-diff/PlanDiffBadge.tsx +++ b/packages/ui/components/plan-diff/PlanDiffBadge.tsx @@ -13,6 +13,18 @@ interface PlanDiffBadgeProps { isActive: boolean; onToggle: () => void; hasPreviousVersion: boolean; + /** + * Optional baseline context for surfaces whose diff baseline is NOT "the + * previous plan revision" — annotate/folder sessions pass "since last + * review" so the counts read against the right baseline (not, e.g., the + * git uncommitted-vs-HEAD numbers shown elsewhere). Rendered as a short + * muted suffix after the counts. Plan review passes nothing and renders + * exactly as before. + */ + baselineLabel?: string; + /** Tooltip override for the inactive state, paired with `baselineLabel` + * (e.g. "Changes since you last reviewed this file"). */ + baselineTooltip?: string; } export const PlanDiffBadge: React.FC = ({ @@ -20,6 +32,8 @@ export const PlanDiffBadge: React.FC = ({ isActive, onToggle, hasPreviousVersion, + baselineLabel, + baselineTooltip, }) => { if (!hasPreviousVersion || !stats) return null; @@ -34,7 +48,11 @@ export const PlanDiffBadge: React.FC = ({ ? "bg-primary/15" : "bg-muted/50 hover:bg-muted" }`} - title={isActive ? "Exit plan diff view" : "Show what changed from previous version"} + title={ + isActive + ? "Exit plan diff view" + : baselineTooltip ?? "Show what changed from previous version" + } > +{stats.additions} @@ -43,6 +61,9 @@ export const PlanDiffBadge: React.FC = ({ -{stats.deletions} + {baselineLabel && ( + {baselineLabel} + )} ); }; From 50a6fcd46b8dd95de214952277b344aa9f8e1cef Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Wed, 22 Jul 2026 08:26:43 -0700 Subject: [PATCH 15/15] fix(editor): exit diff view when the active document loses its baseline MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to the per-document diff baselines: with diff view active on file A, opening a history-less file B left isPlanDiffActive latched on — the diff viewer could not render for B, but the stale flag hid the annotation toolstrip and sticky header until the user pressed Escape. Auto-exit the diff view whenever the active (non-HTML) document has no baseline. The --render-html surface is explicitly gated out: its diff view is driven by htmlDiffHtml with usePlanDiff fed nulls, so hasPreviousVersion is always false there and auto-exiting would kill the HTML diff toggle. Plan review is unaffected — the root document's baseline never goes false mid-session. DOM tests cover the exit, the keep-active document switch, the HTML gate, and the no-baseline activation snap-back. --- .github/workflows/test.yml | 1 + packages/editor/App.tsx | 12 ++ .../editor/hooks/usePlanDiffViewAutoExit.ts | 30 ++++ packages/editor/planDiffAutoExit.test.tsx | 161 ++++++++++++++++++ 4 files changed, 204 insertions(+) create mode 100644 packages/editor/hooks/usePlanDiffViewAutoExit.ts create mode 100644 packages/editor/planDiffAutoExit.test.tsx diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index aeef20e94..0e0971b74 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -66,6 +66,7 @@ jobs: packages/ui/hooks/usePlanDiff.test.tsx packages/ui/hooks/useLinkedDoc.test.tsx packages/ui/components/DocBadges.test.tsx + packages/editor/planDiffAutoExit.test.tsx pi-extension-ai-runtime-windows: # Exercises the Pi extension's Node/jiti server mirror on Windows with an diff --git a/packages/editor/App.tsx b/packages/editor/App.tsx index dc5dd8a71..659fc7d46 100644 --- a/packages/editor/App.tsx +++ b/packages/editor/App.tsx @@ -116,6 +116,7 @@ const DEMO_PLAN_CONTENT = USE_DIFF_DEMO ? DIFF_DEMO_PLAN_CONTENT : DEFAULT_DEMO_PLAN_CONTENT; import { useCheckboxOverrides } from './hooks/useCheckboxOverrides'; +import { usePlanDiffViewAutoExit } from './hooks/usePlanDiffViewAutoExit'; import { AppHeader } from './components/AppHeader'; import { AnnotateAgentTerminalPanel, @@ -804,6 +805,17 @@ const App: React.FC = () => { activeDocDiffFetchers, activeDocFilepath, ); + // Exit diff view when the active document switches to one with no diff + // baseline (e.g. a history-less folder file) — otherwise the stale active + // flag hides the annotation toolstrip until Escape. Gated off HTML surfaces, + // whose diff view is driven by htmlDiffHtml (usePlanDiff is fed nulls there, + // so hasPreviousVersion is always false). See usePlanDiffViewAutoExit. + const exitPlanDiffView = useCallback(() => setIsPlanDiffActive(false), []); + usePlanDiffViewAutoExit( + isPlanDiffActive && !isHtmlSurface, + planDiff.hasPreviousVersion, + exitPlanDiffView, + ); const warnFinishEditingFirst = useCallback((target: 'versions' | 'diff') => { toast('Finish editing first', { description: target === 'versions' diff --git a/packages/editor/hooks/usePlanDiffViewAutoExit.ts b/packages/editor/hooks/usePlanDiffViewAutoExit.ts new file mode 100644 index 000000000..2dcf8947c --- /dev/null +++ b/packages/editor/hooks/usePlanDiffViewAutoExit.ts @@ -0,0 +1,30 @@ +import { useEffect } from 'react'; + +/** + * Exit the plan-diff view when the active document loses its diff baseline — + * e.g. folder annotate: diff view is active on file A, then the user opens a + * history-less file B. The diff viewer can't render for B (no diff blocks), + * but the stale `isPlanDiffActive` flag would otherwise keep the annotation + * toolstrip and sticky header hidden until the user pressed Escape. + * + * Callers must pass `active: false` for surfaces whose diff view is NOT + * driven by usePlanDiff's markdown baseline (the --render-html surface uses + * `htmlDiffHtml` with usePlanDiff fed nulls — auto-exiting there would kill + * the HTML diff toggle immediately). + * + * Safe for plan review: the root document's baseline (`hasPreviousVersion`) + * never goes false while the diff view is open there — versionInfo is stable + * for the session and selectBaseVersion never clears the base plan — so this + * effect only ever fires on document switches. + */ +export function usePlanDiffViewAutoExit( + active: boolean, + hasPreviousVersion: boolean, + exitPlanDiff: () => void, +): void { + useEffect(() => { + if (active && !hasPreviousVersion) { + exitPlanDiff(); + } + }, [active, hasPreviousVersion, exitPlanDiff]); +} diff --git a/packages/editor/planDiffAutoExit.test.tsx b/packages/editor/planDiffAutoExit.test.tsx new file mode 100644 index 000000000..f2f38324f --- /dev/null +++ b/packages/editor/planDiffAutoExit.test.tsx @@ -0,0 +1,161 @@ +/** + * usePlanDiffViewAutoExit + usePlanDiff integration (DOM_TESTS=1) + * + * Mirrors App.tsx's wiring: the diff-view active flag lives in the harness + * (like App's isPlanDiffActive state), usePlanDiff tracks the active + * document via docKey, and usePlanDiffViewAutoExit exits diff view when the + * newly active document carries no baseline — the annotate-folder "diff view + * stuck on after opening a history-less file" quirk. Also pins that a switch + * to a baseline-carrying document does NOT exit, and that the HTML-surface + * gate (active passed as false) suppresses the auto-exit entirely. + */ + +import { afterEach, describe, expect, test } from 'bun:test'; +import React, { act, useState } from 'react'; +import { createRoot, type Root } from 'react-dom/client'; +import { usePlanDiff, type VersionInfo } from '@plannotator/ui/hooks/usePlanDiff'; +import { usePlanDiffViewAutoExit } from './hooks/usePlanDiffViewAutoExit'; + +const hasDom = typeof document !== 'undefined'; + +interface DocState { + key: string; + markdown: string; + previousPlan: string | null; + versionInfo: VersionInfo | null; +} + +interface HarnessApi { + isPlanDiffActive: boolean; + hasPreviousVersion: boolean; + activate: () => void; + switchDoc: (doc: DocState) => void; +} + +const DOC_WITH_BASELINE: DocState = { + key: '/tmp/a.md', + markdown: 'A v2\n', + previousPlan: 'A v1\n', + versionInfo: { version: 2, totalVersions: 2, project: 'test' }, +}; + +const DOC_WITHOUT_BASELINE: DocState = { + key: '/tmp/b.md', + markdown: 'B fresh\n', + previousPlan: null, + versionInfo: null, +}; + +const SECOND_DOC_WITH_BASELINE: DocState = { + key: '/tmp/c.md', + markdown: 'C v3\n', + previousPlan: 'C v2\n', + versionInfo: { version: 3, totalVersions: 3, project: 'test' }, +}; + +let roots: Root[] = []; +let containers: HTMLElement[] = []; + +function Harness({ + apiRef, + htmlSurface = false, +}: { + apiRef: { current: HarnessApi | null }; + htmlSurface?: boolean; +}) { + const [doc, setDoc] = useState(DOC_WITH_BASELINE); + const [isPlanDiffActive, setIsPlanDiffActive] = useState(false); + + const planDiff = usePlanDiff( + doc.markdown, + htmlSurface ? null : doc.previousPlan, + htmlSurface ? null : doc.versionInfo, + undefined, + doc.key, + ); + + usePlanDiffViewAutoExit( + isPlanDiffActive && !htmlSurface, + planDiff.hasPreviousVersion, + () => setIsPlanDiffActive(false), + ); + + apiRef.current = { + isPlanDiffActive, + hasPreviousVersion: planDiff.hasPreviousVersion, + activate: () => setIsPlanDiffActive(true), + switchDoc: setDoc, + }; + return null; +} + +async function mountHarness(htmlSurface = false): Promise<{ current: () => HarnessApi }> { + const container = document.createElement('div'); + document.body.appendChild(container); + containers.push(container); + const root = createRoot(container); + roots.push(root); + const apiRef: { current: HarnessApi | null } = { current: null }; + await act(async () => { + root.render(); + }); + return { + current: () => { + if (!apiRef.current) throw new Error('Harness not mounted'); + return apiRef.current; + }, + }; +} + +afterEach(async () => { + for (const root of roots) await act(async () => root.unmount()); + roots = []; + for (const container of containers) container.remove(); + containers = []; +}); + +describe('usePlanDiffViewAutoExit', () => { + test.skipIf(!hasDom)('exits diff view when the active document switches to one with no baseline', async () => { + const harness = await mountHarness(); + + await act(async () => harness.current().activate()); + expect(harness.current().isPlanDiffActive).toBe(true); + expect(harness.current().hasPreviousVersion).toBe(true); + + await act(async () => harness.current().switchDoc(DOC_WITHOUT_BASELINE)); + expect(harness.current().hasPreviousVersion).toBe(false); + expect(harness.current().isPlanDiffActive).toBe(false); + }); + + test.skipIf(!hasDom)('keeps diff view active when switching to a document that has its own baseline', async () => { + const harness = await mountHarness(); + + await act(async () => harness.current().activate()); + await act(async () => harness.current().switchDoc(SECOND_DOC_WITH_BASELINE)); + + expect(harness.current().hasPreviousVersion).toBe(true); + expect(harness.current().isPlanDiffActive).toBe(true); + }); + + test.skipIf(!hasDom)('never auto-exits on HTML surfaces, where usePlanDiff is fed nulls by design', async () => { + const harness = await mountHarness(true); + + await act(async () => harness.current().activate()); + // hasPreviousVersion is always false here (nulls in), but the caller + // gates `active` off, so the html diff toggle must stay on. + expect(harness.current().hasPreviousVersion).toBe(false); + expect(harness.current().isPlanDiffActive).toBe(true); + }); + + test.skipIf(!hasDom)('stays off for a stable root document with no baseline (plan review first version)', async () => { + const harness = await mountHarness(); + + await act(async () => harness.current().switchDoc(DOC_WITHOUT_BASELINE)); + expect(harness.current().isPlanDiffActive).toBe(false); + + // Activating with no baseline immediately snaps back off — the stuck + // state can never establish itself. + await act(async () => harness.current().activate()); + expect(harness.current().isPlanDiffActive).toBe(false); + }); +});