diff --git a/src/push.ts b/src/push.ts index d4be749..cc70351 100644 --- a/src/push.ts +++ b/src/push.ts @@ -13,14 +13,20 @@ * - A with pageId -> update (restored/copied file; the page already exists). * - M -> update content (collab/Yjs path, SPEC §2/§15.6). * - D -> delete_page (pageId recovered from the PRE-IMAGE meta). - * - R -> rename/move (RECORDED ONLY here; see the TODO below). + * - R -> rename/move (CLASSIFIED here, APPLIED in push #3). + * + * MOVE/RENAME APPLY (push #3) — DONE here. `classifyRenameMoves` (PURE) resolves + * each `renamesMoves` entry into the Docmost op(s) it needs, comparing the PATH- + * derived parent (SPEC §5: the file path is the source of truth for tree + * position, NOT stale `meta.parentPageId`) and the meta title; `applyPushActions` + * then calls `move_page` / `rename_page` (both for a reparent+retitle), or + * records a NO-OP for a cosmetic local-only file-path rename. * * SCOPE OF THIS INCREMENT — what is intentionally NOT here yet (next increment), * left as explicit TODO markers: - * - TODO(next-increment): move/rename APPLY — resolving move-vs-rename and the - * new parentPageId, then calling `move_page` / `rename_page` (SPEC §6/§8). - * `computePushActions` already CLASSIFIES R into `renamesMoves`, and - * `applyPushActions` returns them as `deferred` without any client call. + * - TODO(next): the precise fractional-index `position` for `move_page` + * (SPEC §16). `applyPushActions` passes `position` UNDEFINED for now and the + * client supplies a valid default; computing a key between siblings is next. * - loop-guard PRODUCTION (SPEC §10) — DONE here: `applyPushActions` records, * per applied page, the pushed body hash + the write's `updatedAt` (see * `ApplyPushResult.pushed`) and fast-forwards the `docmost` mirror after a @@ -72,6 +78,104 @@ export interface RenameMoveAction { newPath: string; } +/** + * A CLASSIFIED rename/move (push #3): a `RenameMoveAction` resolved into the + * Docmost op(s) it actually needs. The file PATH is the source of truth for tree + * position (SPEC §5: "истина связи — pageId, не путь" — the path is COSMETIC and + * LOCAL, the page identity is its pageId), so we compare the RESOLVED parent of + * the new path against the resolved parent of the old path, and the title in the + * current meta against the title in the previous meta. Each sub-op is emitted + * ONLY when something real changed: + * - `move` — the resolved parent page changed (reparent in Docmost). A `null` + * `parentPageId` means the new parent is ROOT (the file sits at the space + * root, no enclosing folder). + * - `rename` — the page title changed (a pure title edit in Docmost). + * - `noop` — neither changed: a purely LOCAL file-path rename (same parent, + * same title). The page identity is its pageId, so Docmost is NOT called. + * `move` and `rename` are independent and may BOTH be present (reparent + retitle). + */ +export interface RenameMoveActionClassified { + pageId: string; + oldPath: string; + newPath: string; + /** Present iff the resolved parent changed -> `move_page` (reparent). */ + move?: { parentPageId: string | null }; + /** Present iff the title changed -> `rename_page` (title-only). */ + rename?: { title: string }; + /** True iff neither parent nor title changed (cosmetic local-only rename). */ + noop?: true; +} + +/** + * Injected resolvers for the PURE `classifyRenameMoves` (push #3). Both are PURE + * given a path + side; the real `main` (a follow-up) wires them to the file tree + * (`readFile` for `current`, `git.showFileAtRef` for `prev`), tests pass plain + * lookups. SPEC §5 path-as-truth: + * - `metaAt`: the file's `docmost:meta` at that side (for the title). + * - `resolveParentPageId`: the pageId of the page whose FILE is the parent + * FOLDER's `.md` (one level up from the given path), or `null` for ROOT. + */ +export interface ClassifyRenameMovesDeps { + metaAt: (path: string, side: MetaSide) => DocmostMdMeta | null; + resolveParentPageId: (path: string, side: MetaSide) => string | null; +} + +/** + * PURE classifier for the `renamesMoves` produced by `computePushActions` + * (push #3, SPEC §5/§6/§8). Resolves each `{pageId, oldPath, newPath}` into the + * Docmost op(s) it needs, with NO IO (both resolvers are injected). + * + * SPEC §5 — the file PATH is the source of truth for tree position, NOT the + * (possibly stale) `meta.parentPageId`. So the NEW parent is resolved from + * `newPath`'s enclosing folder, and the OLD parent from `oldPath`'s enclosing + * folder, via `deps.resolveParentPageId`. The title comes from the meta. + * + * For each entry: + * - `newParent = resolveParentPageId(newPath, 'current')`, + * `oldParent = resolveParentPageId(oldPath, 'prev')`. + * - `newTitle = metaAt(newPath,'current')?.title`, + * `oldTitle = metaAt(oldPath,'prev')?.title`. + * - include `move` iff `newParent !== oldParent` (a real reparent), + * - include `rename` iff `newTitle` is a NON-EMPTY string AND differs from + * `oldTitle` (a real title edit; an empty/absent new title is never a rename), + * - if NEITHER applies -> `noop: true` (a cosmetic local-only file-path rename; + * the page is its pageId, so Docmost is not touched). + */ +export function classifyRenameMoves( + renamesMoves: RenameMoveAction[], + deps: ClassifyRenameMovesDeps, +): RenameMoveActionClassified[] { + return renamesMoves.map((rm) => { + const newParent = deps.resolveParentPageId(rm.newPath, "current"); + const oldParent = deps.resolveParentPageId(rm.oldPath, "prev"); + const newTitle = deps.metaAt(rm.newPath, "current")?.title; + const oldTitle = deps.metaAt(rm.oldPath, "prev")?.title; + + const out: RenameMoveActionClassified = { + pageId: rm.pageId, + oldPath: rm.oldPath, + newPath: rm.newPath, + }; + // A reparent: the new path's resolved parent page differs from the old's. + if (newParent !== oldParent) { + out.move = { parentPageId: newParent }; + } + // A title edit: only when there is a real, non-empty new title that changed. + if ( + typeof newTitle === "string" && + newTitle.length > 0 && + newTitle !== oldTitle + ) { + out.rename = { title: newTitle }; + } + // Neither changed -> a purely LOCAL file-path rename; do NOT call Docmost. + if (!out.move && !out.rename) { + out.noop = true; + } + return out; + }); +} + /** The classified set of push actions (PURE output of `computePushActions`). */ export interface PushActions { creates: CreateAction[]; @@ -257,7 +361,7 @@ export const DOCMOST_BRANCH = "docmost"; * Injectable IO for `applyPushActions`. The real `main` (NEXT increment) wires * these to the live client, `node:fs/promises`, and the vault git wrapper; this * increment drives them only through FAKES in tests (no live destructive run). - * - `client`: the create/update/delete subset of `DocmostClient`. + * - `client`: the create/update/delete/move/rename subset of `DocmostClient`. * - `readFile`/`writeFile`: read a changed file's body / write a file back * (by vault-relative path; the applier does not resolve absolute paths so * fakes stay trivial). @@ -268,13 +372,23 @@ export const DOCMOST_BRANCH = "docmost"; export interface ApplyPushDeps { client: Pick< DocmostClient, - "importPageMarkdown" | "createPage" | "deletePage" + | "importPageMarkdown" + | "createPage" + | "deletePage" + | "movePage" + | "renamePage" >; /** Read a changed file's full text by its vault-relative path. */ readFile: (path: string) => Promise; /** Write a file's full text by its vault-relative path. */ writeFile: (path: string, text: string) => Promise; - git: Pick; + /** + * `updateRef` advances `refs/docmost/last-pushed`; `fastForwardBranch` advances + * the `docmost` mirror after a clean push. `showFileAtRef` reads a file's text + * at a ref (used by the move/rename classifier to resolve the PREVIOUS parent + * folder's `.md` at `refs/docmost/last-pushed`, SPEC §5 path-as-truth). + */ + git: Pick; } /** A file whose meta was rewritten with a freshly-assigned pageId (post-create). */ @@ -306,20 +420,38 @@ export interface PushedPageRecord { * refs are NOT advanced when there is any failure, so a re-run retries cleanly. */ export interface PushFailure { - kind: "update" | "create" | "delete"; - /** The pageId for update/delete; absent for a create that never got one. */ + kind: "update" | "create" | "delete" | "move" | "rename"; + /** The pageId for update/delete/move/rename; absent for a never-id'd create. */ pageId?: string; - /** The vault-relative path for create/update; absent for a delete. */ + /** The vault-relative path for create/update/move/rename; absent for delete. */ path?: string; /** The error message captured from the thrown error. */ error: string; } -/** Structured outcome of `applyPushActions` (counts + write-backs + deferred). */ +/** + * A rename/move action that resolved to a NO-OP (push #3, SPEC §5): a purely + * LOCAL file-path rename whose resolved parent AND title are both unchanged. The + * page identity is its pageId and the path is COSMETIC/local-only, so Docmost is + * NOT called — the skip is recorded here (with the reason) for logging. + */ +export interface PushNoop { + pageId: string; + oldPath: string; + newPath: string; + /** Why no Docmost op was emitted (currently always a path-only rename). */ + reason: "path-only-rename"; +} + +/** Structured outcome of `applyPushActions` (counts + write-backs + noops). */ export interface ApplyPushResult { created: number; updated: number; deleted: number; + /** Pages reparented in Docmost via `move_page` (push #3, SPEC §5/§16). */ + moved: number; + /** Pages retitled in Docmost via `rename_page` (push #3, SPEC §5/§6). */ + renamed: number; /** * Files whose `docmost:meta` was rewritten with the pageId Docmost assigned on * create — these now need a FOLLOW-UP commit (the meta on disk changed). The @@ -339,8 +471,12 @@ export interface ApplyPushResult { * (SPEC §12). Non-empty here means the refs were NOT advanced. */ failures: PushFailure[]; - /** Rename/move actions NOT executed this increment (apply is deferred). */ - deferred: RenameMoveAction[]; + /** + * Rename/move actions that resolved to a NO-OP — a purely LOCAL file-path + * rename (same parent, same title). NO Docmost call was made for these (SPEC + * §5: the page is its pageId, the path is local-only). Recorded for logging. + */ + noops: PushNoop[]; /** Diff rows the planner could not classify (carried through for logging). */ skipped: PushActions["skipped"]; /** Whether `refs/docmost/last-pushed` was advanced (only on a CLEAN push). */ @@ -369,7 +505,15 @@ export interface ApplyPushResult { * tracked. The write-back is recorded in `writtenBack` (a follow-up commit * is needed — NEXT increment). * - DELETE: `client.deletePage(pageId)` — soft-delete to Trash (SPEC §8). - * - RENAME/MOVE: NOT executed — returned as `deferred` (NEXT increment). + * - RENAME/MOVE (push #3, SPEC §5/§6/§16): classify each `renamesMoves` entry + * with `classifyRenameMoves` (resolvers read the parent FOLDER's `.md` for + * the parent pageId — path-as-truth — and the meta for the title), then: + * - `move` -> `client.movePage(pageId, parentPageId, position?)` (reparent; + * `position` is UNDEFINED for now — the client supplies a default), + * - `rename` -> `client.renamePage(pageId, title)` (title-only), + * - BOTH -> move (reparent) THEN rename (title), in that order, + * - `noop` -> NO client call; recorded in `noops` (a cosmetic local-only + * file-path rename: the page is its pageId, the path is local, SPEC §5). * * FAIL-SAFE / per-page isolation (SPEC §12 resumability). Each page's operation * is wrapped in its own try/catch: a single failing page is recorded in @@ -408,9 +552,12 @@ export async function applyPushActions( let created = 0; let updated = 0; let deleted = 0; + let moved = 0; + let renamed = 0; const writtenBack: WrittenBackPage[] = []; const pushed: PushedPageRecord[] = []; const failures: PushFailure[] = []; + const noops: PushNoop[] = []; // 1. UPDATES — collab/Yjs write path (SPEC §2/§15.6), never a raw overwrite. // Each update is isolated: a thrown page is recorded and the batch goes on. @@ -489,8 +636,105 @@ export async function applyPushActions( } } - // 4. RENAME/MOVE — DEFERRED (NEXT increment): no client call. Returned as - // `deferred` so the caller can see what still needs the move/rename apply. + // 4. RENAME/MOVE (push #3, SPEC §5/§6/§16). Classify each entry against the + // tree-backed resolvers (the NEW parent comes from the new path's enclosing + // folder `.md`, the OLD parent from the old path's at last-pushed — PATH is + // the truth, not stale `meta.parentPageId`; the title from the meta), then + // apply only the real ops. Each page is isolated like the cases above: a + // thrown op is recorded in `failures` and the batch continues. ORDER for a + // page that needs both: reparent (move) FIRST, then retitle (rename). + if (actions.renamesMoves.length > 0) { + // The classifier is PURE over sync resolvers; the tree reads are async, so + // prefetch every (path, side) lookup it will make into plain tables first. + const parentTable = new Map(); + const metaTable = new Map(); + // A tree read (readFile / git.showFileAtRef) throwing must isolate THAT page + // into `failures`, NOT abort the whole batch (§12 resumability). The helpers + // already swallow their own errors, but this per-entry try/catch keeps the + // batch-isolation invariant holding regardless of future changes to them. + const prefetchFailed = new Set(); + for (const rm of actions.renamesMoves) { + // newParent + newTitle from the CURRENT tree; oldParent + oldTitle from the + // last-pushed pre-image (`prev`). Keyed by `path|side` so duplicates fold. + try { + parentTable.set( + `${rm.newPath}|current`, + await resolveParentPageIdViaTree(deps, rm.newPath, "current"), + ); + parentTable.set( + `${rm.oldPath}|prev`, + await resolveParentPageIdViaTree(deps, rm.oldPath, "prev"), + ); + metaTable.set( + `${rm.newPath}|current`, + await metaAtViaTree(deps, rm.newPath, "current"), + ); + metaTable.set( + `${rm.oldPath}|prev`, + await metaAtViaTree(deps, rm.oldPath, "prev"), + ); + } catch (err: unknown) { + prefetchFailed.add(rm.pageId); + failures.push({ + kind: "move", + pageId: rm.pageId, + path: rm.newPath, + error: errMessage(err), + }); + } + } + const classified = classifyRenameMoves( + actions.renamesMoves.filter((rm) => !prefetchFailed.has(rm.pageId)), + { + metaAt: (path, side) => metaTable.get(`${path}|${side}`) ?? null, + resolveParentPageId: (path, side) => + parentTable.get(`${path}|${side}`) ?? null, + }, + ); + + for (const c of classified) { + if (c.noop) { + // Cosmetic local-only file-path rename — no Docmost op (SPEC §5). + noops.push({ + pageId: c.pageId, + oldPath: c.oldPath, + newPath: c.newPath, + reason: "path-only-rename", + }); + continue; + } + // Track which op is in flight so a failure is attributed to the op that + // ACTUALLY threw: for a page needing both, a move that succeeds then a + // rename that throws must be recorded as `rename`, not `move`. + let failingKind: "move" | "rename" = c.move ? "move" : "rename"; + try { + // Reparent FIRST so the page is in its new tree position, THEN retitle. + if (c.move) { + failingKind = "move"; + // TODO(next): compute a fractional-index position between siblings + // (SPEC §16). `position` is UNDEFINED here; the client supplies a valid + // default. Pass `parentPageId: null` for a move to the space ROOT. + await client.movePage(c.pageId, c.move.parentPageId); + moved++; + } + if (c.rename) { + failingKind = "rename"; + await client.renamePage(c.pageId, c.rename.title); + renamed++; + } + } catch (err: unknown) { + // Isolate the failed page: the op that ACTUALLY threw is recorded so a + // re-run can retry. A move that threw before its rename leaves `rename` + // for the next run (idempotent re-apply); refs are NOT advanced (below). + failures.push({ + kind: failingKind, + pageId: c.pageId, + path: c.newPath, + error: errMessage(err), + }); + } + } + } // 5. Advance the refs ONLY on a CLEAN push (no failures) AND when a pushed // commit is supplied. A partial push must advance NEITHER ref, so a re-run @@ -514,10 +758,12 @@ export async function applyPushActions( created, updated, deleted, + moved, + renamed, writtenBack, pushed, failures, - deferred: actions.renamesMoves, + noops, skipped: actions.skipped, lastPushedAdvanced, docmostFastForward, @@ -529,6 +775,89 @@ function errMessage(err: unknown): string { return err instanceof Error ? err.message : String(err); } +/** + * SPEC §5 path-as-truth: the parent FOLDER's `.md` file for a vault-relative + * (forward-slash) path. `buildVaultLayout` puts a page with children at + * `<...>/Title.md` and nests its children under `<...>/Title/`, so for + * `newPath = /Child.md` the parent page's file is `.md` (the enclosing + * folder, one level up). A path with NO enclosing folder (`Child.md`, at the + * space root) has no parent folder file -> `null` (the parent is ROOT). + */ +export function parentFolderFile(path: string): string | null { + const slash = path.lastIndexOf("/"); + if (slash < 0) return null; // root-level file: no enclosing folder. + return `${path.slice(0, slash)}.md`; +} + +/** + * Build the `resolveParentPageId(path, side)` resolver `classifyRenameMoves` + * needs, reading the PARENT FOLDER's `.md` (SPEC §5 path-as-truth): + * - `current` -> `deps.readFile(.md)` (the live working tree), + * - `prev` -> `git.showFileAtRef('refs/docmost/last-pushed', .md)` (the + * last-pushed pre-image), + * then parse its `docmost:meta` and return that page's pageId. A root-level path + * (no enclosing folder), a missing/unreadable parent file, or a parent file with + * no parseable pageId all resolve to `null` (parent is ROOT / unknown -> + * `parentPageId: null`, SPEC §16 "parentPageId: null -> в корень"). + * + * The IO is async, so this returns an ASYNC resolver; the call sites prefetch the + * parent pageIds (the classifier itself stays pure/sync over a plain table). + */ +async function resolveParentPageIdViaTree( + deps: Pick, + path: string, + side: MetaSide, +): Promise { + const parentFile = parentFolderFile(path); + if (parentFile === null) return null; // root-level: parent is ROOT. + let text: string | null; + try { + text = + side === "current" + ? await deps.readFile(parentFile) + : await deps.git.showFileAtRef(LAST_PUSHED_REF, parentFile); + } catch { + // Parent folder file missing/unreadable at that side -> treat as ROOT. + return null; + } + if (text === null) return null; // showFileAtRef returns null when absent. + try { + const { meta } = parseDocmostMarkdown(text); + return meta?.pageId ?? null; + } catch { + // Unparseable parent meta -> no resolvable parent pageId. + return null; + } +} + +/** + * Resolve the file `docmost:meta` at a side for the rename/move classifier (the + * title comes from here). Mirrors `resolveParentPageIdViaTree`'s IO sides: + * `current` reads the working tree, `prev` reads `refs/docmost/last-pushed`. + * Returns `null` on a missing/unreadable/unparseable file. + */ +async function metaAtViaTree( + deps: Pick, + path: string, + side: MetaSide, +): Promise { + let text: string | null; + try { + text = + side === "current" + ? await deps.readFile(path) + : await deps.git.showFileAtRef(LAST_PUSHED_REF, path); + } catch { + return null; + } + if (text === null) return null; + try { + return parseDocmostMarkdown(text).meta ?? null; + } catch { + return null; + } +} + /** * Pull an `updatedAt` out of a create/update client result, if present. The * shape is `{ data: { updatedAt? }, ... }` (createPage) or a flatter object; diff --git a/test/apply-push-actions.test.ts b/test/apply-push-actions.test.ts index 01e6264..bb1bd6c 100644 --- a/test/apply-push-actions.test.ts +++ b/test/apply-push-actions.test.ts @@ -33,6 +33,18 @@ function makeClient(opts?: { createId?: string }) { }), ), deletePage: vi.fn(async (_pageId: string) => ({ success: true })), + movePage: vi.fn( + async ( + _pageId: string, + _parentPageId: string | null, + _position?: string, + ) => ({ success: true }), + ), + renamePage: vi.fn(async (pageId: string, title: string) => ({ + success: true, + pageId, + title, + })), }; return client; } @@ -42,9 +54,14 @@ function makeClient(opts?: { createId?: string }) { * (advance the `docmost` mirror, the loop-close). `ffResult` configures what the * ff returns (default a successful advance). */ -function makeGit(opts?: { ffResult?: { ok: boolean; reason?: string } }) { +function makeGit(opts?: { + ffResult?: { ok: boolean; reason?: string }; + /** Pre-image tree at `refs/docmost/last-pushed` (path -> text). */ + prevTree?: Record; +}) { const updateRefCalls: { ref: string; target: string }[] = []; const ffCalls: { branch: string; toCommit: string }[] = []; + const prevTree = opts?.prevTree ?? {}; const git = { updateRef: vi.fn(async (ref: string, target: string) => { updateRefCalls.push({ ref, target }); @@ -53,6 +70,11 @@ function makeGit(opts?: { ffResult?: { ok: boolean; reason?: string } }) { ffCalls.push({ branch, toCommit }); return opts?.ffResult ?? { ok: true }; }), + // The move/rename classifier reads the PREVIOUS parent folder's `.md` at + // refs/docmost/last-pushed via this; `null` when absent there (SPEC §5). + showFileAtRef: vi.fn(async (_ref: string, path: string) => + path in prevTree ? prevTree[path] : null, + ), }; return { git, updateRefCalls, ffCalls }; } @@ -189,23 +211,237 @@ describe('applyPushActions — delete (soft-delete to Trash, SPEC §8)', () => { }); }); -describe('applyPushActions — rename/move is DEFERRED (NEXT increment)', () => { - it('returns renames/moves as `deferred` with NO client call', async () => { - const client = makeClient(); - const { git } = makeGit(); - const fs = makeFs(); +// FS→Docmost push #3 (SPEC §5/§6/§16): the move/rename APPLY. The classifier +// resolves the parent from the FILE PATH (the enclosing folder's `.md`), not +// stale `meta.parentPageId`, then `applyPushActions` calls move_page / rename_page +// (both for a reparent+retitle) or records a path-only NO-OP with NO client call. + +/** + * Helper: a self-contained file with the given pageId + title in its meta. Used + * both to seed the working tree (fs) and the prev tree (git.showFileAtRef). + */ +function fileWith(meta: { pageId: string; title?: string }): string { + return serializeDocmostMarkdownBody( + { version: 1, pageId: meta.pageId, ...(meta.title ? { title: meta.title } : {}) }, + 'body', + ); +} + +describe('applyPushActions — move (parent changed, title same; SPEC §5/§16)', () => { + it('calls movePage(pageId, newParent) and NOT renamePage', async () => { + // The page moved from the space root (Doc.md) under a folder (Parent/Doc.md). + // The new parent page's file is `Parent.md`; its meta carries the parent id. + const client = makeClient(); + const { git } = makeGit({ + // Prev pre-image: the file used to sit at the root (parent ROOT). + prevTree: { 'Doc.md': fileWith({ pageId: 'p-mv', title: 'Doc' }) }, + }); + const fs = makeFs({ + // Current tree: the moved file + its new parent folder's `.md`. + 'Parent/Doc.md': fileWith({ pageId: 'p-mv', title: 'Doc' }), + 'Parent.md': fileWith({ pageId: 'parent-id', title: 'Parent' }), + }); - const rm = { pageId: 'p-mv', oldPath: 'Old.md', newPath: 'New.md' }; const res = await applyPushActions( deps(client, git, fs), - actions({ renamesMoves: [rm] }), + actions({ + renamesMoves: [ + { pageId: 'p-mv', oldPath: 'Doc.md', newPath: 'Parent/Doc.md' }, + ], + }), ); - expect(res.deferred).toEqual([rm]); - // NOTHING was pushed for the move this increment. - expect(client.importPageMarkdown).not.toHaveBeenCalled(); - expect(client.createPage).not.toHaveBeenCalled(); - expect(client.deletePage).not.toHaveBeenCalled(); + expect(res.moved).toBe(1); + expect(res.renamed).toBe(0); + expect(client.movePage).toHaveBeenCalledTimes(1); + // Reparented under `parent-id`; position left UNDEFINED (client default). + expect(client.movePage).toHaveBeenCalledWith('p-mv', 'parent-id'); + expect(client.renamePage).not.toHaveBeenCalled(); + expect(res.noops).toEqual([]); + }); +}); + +describe('applyPushActions — move-to-root (newParent null; SPEC §16)', () => { + it('calls movePage(pageId, null) when the file lands at the space root', async () => { + const client = makeClient(); + const { git } = makeGit({ + // Prev: the file used to live under `Parent/`, so its old parent is the + // page whose file is `Parent.md` (parent-id). + prevTree: { + 'Parent/Doc.md': fileWith({ pageId: 'p-mv', title: 'Doc' }), + 'Parent.md': fileWith({ pageId: 'parent-id', title: 'Parent' }), + }, + }); + // Current: the file is now at the root -> no enclosing folder -> parent ROOT. + const fs = makeFs({ 'Doc.md': fileWith({ pageId: 'p-mv', title: 'Doc' }) }); + + const res = await applyPushActions( + deps(client, git, fs), + actions({ + renamesMoves: [ + { pageId: 'p-mv', oldPath: 'Parent/Doc.md', newPath: 'Doc.md' }, + ], + }), + ); + + expect(res.moved).toBe(1); + expect(client.movePage).toHaveBeenCalledWith('p-mv', null); + expect(client.renamePage).not.toHaveBeenCalled(); + }); +}); + +describe('applyPushActions — rename (same parent, title changed; SPEC §5/§6)', () => { + it('calls renamePage(pageId, title) and NOT movePage', async () => { + // Same enclosing folder on both sides (parent unchanged), only the title + // changed in meta -> a pure rename. + const client = makeClient(); + const { git } = makeGit({ + prevTree: { + 'Folder/Old.md': fileWith({ pageId: 'p-rn', title: 'Old Title' }), + 'Folder.md': fileWith({ pageId: 'folder-id', title: 'Folder' }), + }, + }); + const fs = makeFs({ + 'Folder/New.md': fileWith({ pageId: 'p-rn', title: 'New Title' }), + 'Folder.md': fileWith({ pageId: 'folder-id', title: 'Folder' }), + }); + + const res = await applyPushActions( + deps(client, git, fs), + actions({ + renamesMoves: [ + { pageId: 'p-rn', oldPath: 'Folder/Old.md', newPath: 'Folder/New.md' }, + ], + }), + ); + + expect(res.renamed).toBe(1); + expect(res.moved).toBe(0); + expect(client.renamePage).toHaveBeenCalledTimes(1); + expect(client.renamePage).toHaveBeenCalledWith('p-rn', 'New Title'); + expect(client.movePage).not.toHaveBeenCalled(); + }); +}); + +describe('applyPushActions — both (reparent + retitle; move THEN rename)', () => { + it('calls movePage first, then renamePage', async () => { + const callOrder: string[] = []; + const client = makeClient(); + client.movePage.mockImplementation(async () => { + callOrder.push('move'); + return { success: true }; + }); + client.renamePage.mockImplementation(async (pageId: string, title: string) => { + callOrder.push('rename'); + return { success: true, pageId, title }; + }); + const { git } = makeGit({ + // Prev: at root (parent ROOT) with the old title. + prevTree: { 'Old.md': fileWith({ pageId: 'p-x', title: 'Old' }) }, + }); + const fs = makeFs({ + // Current: under a new folder AND retitled. + 'NewParent/New.md': fileWith({ pageId: 'p-x', title: 'New' }), + 'NewParent.md': fileWith({ pageId: 'np-id', title: 'NewParent' }), + }); + + const res = await applyPushActions( + deps(client, git, fs), + actions({ + renamesMoves: [ + { pageId: 'p-x', oldPath: 'Old.md', newPath: 'NewParent/New.md' }, + ], + }), + ); + + expect(res.moved).toBe(1); + expect(res.renamed).toBe(1); + expect(client.movePage).toHaveBeenCalledWith('p-x', 'np-id'); + expect(client.renamePage).toHaveBeenCalledWith('p-x', 'New'); + // Order matters: reparent FIRST, then retitle. + expect(callOrder).toEqual(['move', 'rename']); + }); +}); + +describe('applyPushActions — noop (path-only rename; NO Docmost call; SPEC §5)', () => { + it('calls NEITHER movePage NOR renamePage and records the noop', async () => { + // Same enclosing folder AND same title on both sides: a purely LOCAL file + // rename. The page is its pageId; the path is cosmetic -> Docmost untouched. + const client = makeClient(); + const { git } = makeGit({ + prevTree: { + 'Folder/A.md': fileWith({ pageId: 'p-noop', title: 'Same' }), + 'Folder.md': fileWith({ pageId: 'folder-id', title: 'Folder' }), + }, + }); + const fs = makeFs({ + 'Folder/B.md': fileWith({ pageId: 'p-noop', title: 'Same' }), + 'Folder.md': fileWith({ pageId: 'folder-id', title: 'Folder' }), + }); + + const res = await applyPushActions( + deps(client, git, fs), + actions({ + renamesMoves: [ + { pageId: 'p-noop', oldPath: 'Folder/A.md', newPath: 'Folder/B.md' }, + ], + }), + ); + + expect(res.moved).toBe(0); + expect(res.renamed).toBe(0); + // ZERO Docmost calls for a cosmetic rename. + expect(client.movePage).not.toHaveBeenCalled(); + expect(client.renamePage).not.toHaveBeenCalled(); + expect(res.noops).toEqual([ + { + pageId: 'p-noop', + oldPath: 'Folder/A.md', + newPath: 'Folder/B.md', + reason: 'path-only-rename', + }, + ]); + }); +}); + +describe('applyPushActions — move whose client call throws (SPEC §12 isolation)', () => { + it('isolates the failure into `failures` and does NOT advance the refs', async () => { + const client = makeClient(); + client.movePage.mockImplementation(async () => { + throw new Error('move boom'); + }); + const { git, updateRefCalls, ffCalls } = makeGit({ + prevTree: { 'Doc.md': fileWith({ pageId: 'p-mv', title: 'Doc' }) }, + }); + const fs = makeFs({ + 'Parent/Doc.md': fileWith({ pageId: 'p-mv', title: 'Doc' }), + 'Parent.md': fileWith({ pageId: 'parent-id', title: 'Parent' }), + }); + + const res = await applyPushActions( + deps(client, git, fs), + actions({ + renamesMoves: [ + { pageId: 'p-mv', oldPath: 'Doc.md', newPath: 'Parent/Doc.md' }, + ], + }), + 'sha-move-fail', + ); + + expect(res.moved).toBe(0); + expect(res.failures).toEqual([ + { + kind: 'move', + pageId: 'p-mv', + path: 'Parent/Doc.md', + error: 'move boom', + }, + ]); + // A failure means the refs are NOT advanced — a re-run retries cleanly (§12). + expect(res.lastPushedAdvanced).toBe(false); + expect(updateRefCalls).toEqual([]); + expect(ffCalls).toEqual([]); + expect(git.updateRef).not.toHaveBeenCalled(); }); }); diff --git a/test/classify-rename-moves.test.ts b/test/classify-rename-moves.test.ts new file mode 100644 index 0000000..a02f351 --- /dev/null +++ b/test/classify-rename-moves.test.ts @@ -0,0 +1,263 @@ +import { describe, expect, it } from 'vitest'; +import { classifyRenameMoves } from '../src/push.js'; +import type { + ClassifyRenameMovesDeps, + MetaSide, + RenameMoveAction, +} from '../src/push.js'; +import type { DocmostMdMeta } from '../packages/docmost-client/src/lib/markdown-document.js'; + +// FS→Docmost push #3 (SPEC §5/§6/§16). `classifyRenameMoves` is the PURE half of +// the move/rename apply: it resolves each `{pageId, oldPath, newPath}` into the +// Docmost op(s) it needs, with NO IO (both resolvers are injected). The key +// design (SPEC §5) is that the file PATH is the source of truth for tree +// position — the NEW parent comes from the new path, the OLD parent from the old +// path — and the title comes from the meta. An op is emitted ONLY when something +// really changed; a path-only rename (same parent + same title) is a noop and +// NEVER calls Docmost. + +/** Build `metaAt` from a `path|side -> meta` table. */ +function metaTable( + table: Record, +): (path: string, side: MetaSide) => DocmostMdMeta | null { + return (path, side) => { + const key = `${path}|${side}`; + return key in table ? table[key] : null; + }; +} + +/** Build `resolveParentPageId` from a `path|side -> parentPageId|null` table. */ +function parentTable( + table: Record, +): (path: string, side: MetaSide) => string | null { + return (path, side) => { + const key = `${path}|${side}`; + return key in table ? table[key] : null; + }; +} + +function deps( + metas: Record, + parents: Record, +): ClassifyRenameMovesDeps { + return { + metaAt: metaTable(metas), + resolveParentPageId: parentTable(parents), + }; +} + +function meta(partial: Partial): DocmostMdMeta { + return { version: 1, ...partial }; +} + +describe('classifyRenameMoves — move-only (parent changed, title same)', () => { + it('emits move (new parent) and NO rename', () => { + const rms: RenameMoveAction[] = [ + { pageId: 'p1', oldPath: 'Doc.md', newPath: 'Parent/Doc.md' }, + ]; + const out = classifyRenameMoves( + rms, + deps( + { + // Same title on both sides. + 'Parent/Doc.md|current': meta({ title: 'Doc' }), + 'Doc.md|prev': meta({ title: 'Doc' }), + }, + { + // Parent changed: root (null) -> 'parent-id'. + 'Parent/Doc.md|current': 'parent-id', + 'Doc.md|prev': null, + }, + ), + ); + expect(out).toEqual([ + { + pageId: 'p1', + oldPath: 'Doc.md', + newPath: 'Parent/Doc.md', + move: { parentPageId: 'parent-id' }, + }, + ]); + expect(out[0].rename).toBeUndefined(); + expect(out[0].noop).toBeUndefined(); + }); +}); + +describe('classifyRenameMoves — rename-only (same parent, title changed)', () => { + it('emits rename (new title) and NO move', () => { + const rms: RenameMoveAction[] = [ + { pageId: 'p2', oldPath: 'Folder/Old.md', newPath: 'Folder/New.md' }, + ]; + const out = classifyRenameMoves( + rms, + deps( + { + 'Folder/New.md|current': meta({ title: 'New Title' }), + 'Folder/Old.md|prev': meta({ title: 'Old Title' }), + }, + { + // Same parent on both sides. + 'Folder/New.md|current': 'folder-id', + 'Folder/Old.md|prev': 'folder-id', + }, + ), + ); + expect(out).toEqual([ + { + pageId: 'p2', + oldPath: 'Folder/Old.md', + newPath: 'Folder/New.md', + rename: { title: 'New Title' }, + }, + ]); + expect(out[0].move).toBeUndefined(); + expect(out[0].noop).toBeUndefined(); + }); +}); + +describe('classifyRenameMoves — both (parent AND title changed)', () => { + it('emits BOTH move and rename', () => { + const rms: RenameMoveAction[] = [ + { pageId: 'p3', oldPath: 'Old.md', newPath: 'NewParent/New.md' }, + ]; + const out = classifyRenameMoves( + rms, + deps( + { + 'NewParent/New.md|current': meta({ title: 'New' }), + 'Old.md|prev': meta({ title: 'Old' }), + }, + { + 'NewParent/New.md|current': 'np-id', + 'Old.md|prev': null, + }, + ), + ); + expect(out).toEqual([ + { + pageId: 'p3', + oldPath: 'Old.md', + newPath: 'NewParent/New.md', + move: { parentPageId: 'np-id' }, + rename: { title: 'New' }, + }, + ]); + expect(out[0].noop).toBeUndefined(); + }); +}); + +describe('classifyRenameMoves — noop (path-only rename, same parent + title)', () => { + it('emits noop and NEITHER move NOR rename (SPEC §5: page is its pageId)', () => { + const rms: RenameMoveAction[] = [ + { pageId: 'p4', oldPath: 'Folder/A.md', newPath: 'Folder/B.md' }, + ]; + const out = classifyRenameMoves( + rms, + deps( + { + 'Folder/B.md|current': meta({ title: 'Same' }), + 'Folder/A.md|prev': meta({ title: 'Same' }), + }, + { + 'Folder/B.md|current': 'folder-id', + 'Folder/A.md|prev': 'folder-id', + }, + ), + ); + expect(out).toEqual([ + { + pageId: 'p4', + oldPath: 'Folder/A.md', + newPath: 'Folder/B.md', + noop: true, + }, + ]); + expect(out[0].move).toBeUndefined(); + expect(out[0].rename).toBeUndefined(); + }); +}); + +describe('classifyRenameMoves — move-to-root (newParent null)', () => { + it('emits move with parentPageId null when the file lands at the space root', () => { + const rms: RenameMoveAction[] = [ + { pageId: 'p5', oldPath: 'Parent/Doc.md', newPath: 'Doc.md' }, + ]; + const out = classifyRenameMoves( + rms, + deps( + { + 'Doc.md|current': meta({ title: 'Doc' }), + 'Parent/Doc.md|prev': meta({ title: 'Doc' }), + }, + { + // New parent is ROOT (null), old parent was 'parent-id'. + 'Doc.md|current': null, + 'Parent/Doc.md|prev': 'parent-id', + }, + ), + ); + expect(out).toEqual([ + { + pageId: 'p5', + oldPath: 'Parent/Doc.md', + newPath: 'Doc.md', + move: { parentPageId: null }, + }, + ]); + expect(out[0].rename).toBeUndefined(); + expect(out[0].noop).toBeUndefined(); + }); +}); + +describe('classifyRenameMoves — title guards', () => { + it('an EMPTY new title is NOT a rename (even if it differs from old)', () => { + const rms: RenameMoveAction[] = [ + { pageId: 'p6', oldPath: 'Folder/A.md', newPath: 'Folder/B.md' }, + ]; + const out = classifyRenameMoves( + rms, + deps( + { + // New title is empty -> never a rename; same parent -> overall noop. + 'Folder/B.md|current': meta({ title: '' }), + 'Folder/A.md|prev': meta({ title: 'Had A Title' }), + }, + { + 'Folder/B.md|current': 'folder-id', + 'Folder/A.md|prev': 'folder-id', + }, + ), + ); + expect(out[0].rename).toBeUndefined(); + expect(out[0].move).toBeUndefined(); + expect(out[0].noop).toBe(true); + }); + + it('a missing new meta is NOT a rename; a parent change still yields a move', () => { + const rms: RenameMoveAction[] = [ + { pageId: 'p7', oldPath: 'Doc.md', newPath: 'Parent/Doc.md' }, + ]; + const out = classifyRenameMoves( + rms, + deps( + { + // No current meta entry at all (resolver returns null). + 'Doc.md|prev': meta({ title: 'Doc' }), + }, + { + 'Parent/Doc.md|current': 'parent-id', + 'Doc.md|prev': null, + }, + ), + ); + expect(out[0].move).toEqual({ parentPageId: 'parent-id' }); + expect(out[0].rename).toBeUndefined(); + expect(out[0].noop).toBeUndefined(); + }); +}); + +describe('classifyRenameMoves — empty input', () => { + it('returns an empty array for no rename/move entries', () => { + expect(classifyRenameMoves([], deps({}, {}))).toEqual([]); + }); +});