diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 6b9c646c2..0e0971b74 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -63,6 +63,10 @@ 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 + 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/apps/pi-extension/server/annotate-history.test.ts b/apps/pi-extension/server/annotate-history.test.ts new file mode 100644 index 000000000..e069544df --- /dev/null +++ b/apps/pi-extension/server/annotate-history.test.ts @@ -0,0 +1,425 @@ +/** + * 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 { 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.ts"; +import { deriveAnnotateHistorySlug } from "../generated/annotate-history.ts"; +import { getPlannotatorDataDir } from "../generated/data-dir.ts"; + +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; + }); + + // 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 { + 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"); + 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 }; + }; + expect(firstJson.markdown).toBe("V1\n"); + expect(firstJson.previousPlan).toBeNull(); + expect(firstJson.versionInfo).toEqual({ version: 1, totalVersions: 1, project }); + // 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. + 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("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"); + 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(); + } + }); +}); diff --git a/apps/pi-extension/server/reference.ts b/apps/pi-extension/server/reference.ts index a69a4e6f4..ca1c236d8 100644 --- a/apps/pi-extension/server/reference.ts +++ b/apps/pi-extension/server/reference.ts @@ -50,16 +50,50 @@ import { readSourceFileSnapshot, resolveExistingSourceSaveFile, } from "../generated/source-save-node.ts"; +import type { AnnotateHistoryResult } from "../generated/annotate-history.ts"; 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; +// 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; 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, + * 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 + * 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) => FolderAnnotateHistory | null; + }; } interface HandleDocExistsOptions { @@ -96,6 +130,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 +221,17 @@ function resolveMarkdownFileFromAllowedRoots(input: string, roots: string[]): Ro return { kind: "not_found", input }; } +type DocOptionsResult = T & { + sourceSave?: SourceSaveCapability; + previousPlan?: string | null; + versionInfo?: AnnotateHistoryResult["versionInfo"]; +}; + function applyDocOptions>( data: T, options: HandleDocOptions = {}, sourceSnapshot?: SourceFileSnapshot, -): T & { sourceSave?: SourceSaveCapability } { +): DocOptionsResult { const next: Record = { ...data }; if ( typeof next.rawHtml === "string" && @@ -173,16 +240,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 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 ( + options.annotateHistory && + typeof data.filepath === "string" && + data.renderAs === "markdown" && + data.isConverted !== true && + typeof data.markdown === "string" && + isAnnotatableTextPath(data.filepath) + ) { + const history = options.annotateHistory.compute(data.filepath, data.markdown); + if (history) { + next.previousPlan = history.previousPlan; + next.versionInfo = history.versionInfo; + } + } 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 +279,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 +290,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 768f6540a..e60936136 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.ts"; -import { saveToHistory, getPlanVersion, getVersionCount, listVersions } from "../generated/storage.ts"; +import { getPlanVersion, getVersionCount, listVersions } from "../generated/storage.ts"; +import { computeAnnotateHistory, deriveAnnotateHistorySlug, type AnnotateHistoryResult } from "../generated/annotate-history.ts"; import { htmlDiff } from "../generated/html-diff.ts"; import { saveConfig, detectGitUser, getServerConfig, loadConfig, resolveSharingEnabled, resolveAnnotateHistory, type PromptRuntime } from "../generated/config.ts"; import { getAnnotateFileFeedbackTemplate, getAnnotateMessageFeedbackTemplate } from "../generated/prompts.ts"; @@ -42,6 +43,8 @@ import { handleObsidianVaultsRequest, handleObsidianFilesRequest, handleObsidianDocRequest, + resolveAllowedDocPath, + type FolderAnnotateHistory, } from "./reference.ts"; import { handleFileBrowserStreamRequest } from "./file-browser-watch.ts"; import { resolveUserPath, warmFileListCache } from "../generated/resolve-file.ts"; @@ -221,56 +224,43 @@ 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. + // 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 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; + } + // Detect repo info (cached for this session) const repoInfo = getRepoInfo(); @@ -452,9 +442,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; @@ -462,7 +477,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; @@ -470,6 +485,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; @@ -557,6 +591,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 b31cf5d4d..845197618 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 guide-store 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 guide-store 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 diff --git a/packages/editor/App.tsx b/packages/editor/App.tsx index 3bebfc258..f42a05dec 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'; @@ -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, @@ -689,36 +690,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, @@ -792,6 +763,84 @@ 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, + ); + // 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' + ? '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. @@ -4051,7 +4100,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} @@ -4111,8 +4160,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} @@ -4183,6 +4232,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} @@ -4421,10 +4472,12 @@ 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} + 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/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); + }); +}); diff --git a/packages/server/annotate.test.ts b/packages/server/annotate.test.ts index 86b3034c4..ac4e877ba 100644 --- a/packages/server/annotate.test.ts +++ b/packages/server/annotate.test.ts @@ -14,11 +14,13 @@ * 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"; +import { deriveAnnotateHistorySlug } from "@plannotator/shared/annotate-history"; +import { getPlannotatorDataDir } from "@plannotator/shared/data-dir"; const MINIMAL_HTML = "Plannotator"; @@ -503,3 +505,480 @@ 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. + // 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 { + 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"); + 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 }; + }; + expect(firstJson.markdown).toBe("V1\n"); + expect(firstJson.previousPlan).toBeNull(); + expect(firstJson.versionInfo).toEqual({ version: 1, totalVersions: 1, project }); + // 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. + 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 }; + }; + expect(json.previousPlan).toBeNull(); + expect(json.versionInfo).toEqual({ version: 1, totalVersions: 1, project }); + // diffCurrent is intentionally not propagated on the folder /api/doc path. + expect("diffCurrent" in json).toBe(false); + } 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("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"); + 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(); + } + }); +}); diff --git a/packages/server/annotate.ts b/packages/server/annotate.ts index 31ec9a652..4989e0b95 100644 --- a/packages/server/annotate.ts +++ b/packages/server/annotate.ts @@ -15,11 +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, type FolderAnnotateHistory } from "./reference-handlers"; 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, 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,55 +166,42 @@ 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; + const annotateHistoryEnabled = resolveAnnotateHistory(loadConfig()); + let annotateHistory: AnnotateHistoryResult | null = null; { const historyContent = renderHtml && rawHtml ? rawHtml : markdown; const eligible = 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 + // 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); } } + + // 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. + // 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 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; + } const draftSource = mode === "annotate-folder" && folderPath ? `folder:${resolvePath(folderPath)}` @@ -421,16 +409,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 }); } @@ -438,7 +449,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: [] }); } @@ -525,6 +553,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..b9bef7779 100644 --- a/packages/server/reference-handlers.ts +++ b/packages/server/reference-handlers.ts @@ -38,16 +38,49 @@ import { readSourceFileSnapshot, resolveExistingSourceSaveFile, } from "@plannotator/shared/source-save-node"; +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 --- +// 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; 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, + * 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 + * 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) => FolderAnnotateHistory | null; + }; } interface HandleDocExistsOptions { @@ -84,6 +117,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 +207,17 @@ function resolveMarkdownFileFromAllowedRoots(input: string, roots: string[]): Ro return { kind: "not_found", input }; } +type DocOptionsResult = T & { + sourceSave?: SourceSaveCapability; + previousPlan?: string | null; + versionInfo?: AnnotateHistoryResult["versionInfo"]; +}; + function applyDocOptions>( data: T, options: HandleDocOptions = {}, sourceSnapshot?: SourceFileSnapshot, -): T & { sourceSave?: SourceSaveCapability } { +): DocOptionsResult { const next: Record = { ...data }; if ( typeof next.rawHtml === "string" && @@ -161,16 +226,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 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 ( + options.annotateHistory && + typeof data.filepath === "string" && + data.renderAs === "markdown" && + data.isConverted !== true && + typeof data.markdown === "string" && + isAnnotatableTextPath(data.filepath) + ) { + const history = options.annotateHistory.compute(data.filepath, data.markdown); + if (history) { + next.previousPlan = history.previousPlan; + next.versionInfo = history.versionInfo; + } + } 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 +265,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 +276,7 @@ function applyDocOptions>( return { ...next, sourceSave, - } as T & { sourceSave?: SourceSaveCapability }; + } as DocOptionsResult; } function docJson(data: Record, options?: HandleDocOptions, sourceSnapshot?: SourceFileSnapshot): Response { 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 c383043ba..a769f59e1 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", 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 2222800ef..79c25c01d 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'; @@ -31,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; @@ -50,6 +59,8 @@ export const DocBadges: React.FC = ({ isPlanDiffActive, hasPreviousVersion, onPlanDiffToggle, + planDiffBaselineLabel, + planDiffBaselineTooltip, showDemoBadge, archiveInfo, linkedDocInfo, @@ -64,10 +75,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,12 +137,14 @@ export const DocBadges: React.FC = ({ )} - {onPlanDiffToggle && !linkedDocInfo && ( + {onPlanDiffToggle && ( )} 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} + )} ); }; 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..fd3dd62f3 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,43 @@ 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 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. + */ + previousPlan?: string | null; + versionInfo?: VersionInfo | null; +} + +/** + * 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 +111,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 +153,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 +189,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 +225,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 +322,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 +354,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 +454,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 +555,7 @@ export function useLinkedDoc(options: UseLinkedDocOptions): UseLinkedDocReturn { snapshotSession, restoreSession, docAnnotationCount, + diffPreviousPlan: linkedDoc?.previousPlan ?? null, + diffVersionInfo: linkedDoc?.versionInfo ?? null, }; } diff --git a/packages/ui/hooks/usePlanDiff.test.tsx b/packages/ui/hooks/usePlanDiff.test.tsx new file mode 100644 index 000000000..9a85ade53 --- /dev/null +++ b/packages/ui/hooks/usePlanDiff.test.tsx @@ -0,0 +1,388 @@ +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 — 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' }; + 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..a57d6fceb 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,22 @@ 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 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 { const fetchVersionImpl = fetchers?.fetchVersion ?? defaultFetchVersion; const fetchVersionsImpl = fetchers?.fetchVersions ?? defaultFetchVersions; @@ -118,6 +133,62 @@ export function usePlanDiff( } }, [versionInfo]); + // 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; + + 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); + 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;