diff --git a/packages/git-sync/src/engine/push.ts b/packages/git-sync/src/engine/push.ts index 96e9c5c3..b404b17c 100644 --- a/packages/git-sync/src/engine/push.ts +++ b/packages/git-sync/src/engine/push.ts @@ -27,11 +27,8 @@ * `DocmostClient`; the upstream CLI `main()` entry point is dropped (the gitmost * server drives the engine in-process). Engine LOGIC is byte-identical. */ -import { - parseDocmostMarkdown, - serializeDocmostMarkdownBody, - type DocmostMdMeta, -} from "../lib/index"; +import { type DocmostMdMeta } from "../lib/index"; +import { parsePageFile, serializePageFile } from "../lib/page-file"; import type { GitSyncClient } from "./client.types"; import type { DiffEntry } from "./git"; import { VaultGit, DEFAULT_BRANCH } from "./git"; @@ -449,6 +446,12 @@ export interface ApplyPushDeps { readFile: (path: string) => Promise; /** Write a file's full text by its vault-relative path. */ writeFile: (path: string, text: string) => Promise; + /** + * The Docmost spaceId this vault mirrors. A CREATE targets this space (the + * native file carries no spaceId — every file in the vault belongs to it), and + * it backs the synthetic native meta the classifier reads. + */ + spaceId: string; /** * `updateRef` advances `refs/docmost/last-pushed`; `fastForwardBranch` advances * the `docmost` mirror after a clean push. `showFileAtRef` reads a file's text @@ -630,25 +633,27 @@ export async function applyPushActions( // Each update is isolated: a thrown page is recorded and the batch goes on. for (const u of actions.updates) { try { - const fullMarkdown = await deps.readFile(u.path); + // Push the CLEAN body only (no `gitmost_id` frontmatter): the frontmatter + // is engine metadata, never page content. The server converts the markdown + // it receives verbatim, so stripping here keeps the id out of Docmost. + const body = parsePageFile(await deps.readFile(u.path)).body; // The last-synced version of this file (pre-image) is the common ancestor // for a 3-way merge against the live page, so concurrent human edits are - // not clobbered (review #5). Null when the file is new at last-pushed. - const baseMarkdown = await deps.git.showFileAtRef( - LAST_PUSHED_REF, - u.path, - ); + // not clobbered (review #5). Null when the file is new at last-pushed. Its + // body is stripped the SAME way so the merge compares body-to-body. + const baseFull = await deps.git.showFileAtRef(LAST_PUSHED_REF, u.path); + const baseMarkdown = baseFull === null ? null : parsePageFile(baseFull).body; const result = await client.importPageMarkdown( u.pageId, - fullMarkdown, + body, baseMarkdown, ); updated++; - // §10 loop-guard data: hash the body we pushed + capture `updatedAt`. + // §10 loop-guard data: hash the BODY we pushed + capture `updatedAt`. pushed.push({ pageId: u.pageId, ...extractUpdatedAt(result), - bodyHash: bodyHash(fullMarkdown), + bodyHash: bodyHash(body), }); } catch (err: unknown) { failures.push({ @@ -666,32 +671,33 @@ export async function applyPushActions( for (const c of actions.creates) { try { const text = await deps.readFile(c.path); - const { meta, body } = parseDocmostMarkdown(text); - // Derive create args from the file's current meta. A new local file may - // have partial meta (e.g. title/spaceId only); spaceId is required by - // Docmost (the planner already guards a create against a missing spaceId). - const title = meta?.title ?? ""; - const spaceId = meta?.spaceId ?? ""; - const parentPageId = meta?.parentPageId ?? undefined; - const result = await client.createPage(title, body, spaceId, parentPageId); + const { body } = parsePageFile(text); + // Derive create args from the PATH (native-Obsidian, SPEC §5): title from + // the filename, parent from the enclosing folder's folder-note, space from + // the run (the vault's space). `parentPageId: null` -> created at ROOT. + const title = titleFromPath(c.path); + const parentPageId = + (await resolveParentPageIdViaTree(deps, c.path, "current")) ?? undefined; + const result = await client.createPage( + title, + body, + deps.spaceId, + parentPageId, + ); // `createPage` returns `{ data: { id, ... }, success }`; the assigned // pageId is at `result.data.id`. const assignedPageId: string | undefined = result?.data?.id; if (assignedPageId) { - // Re-serialize the file with the pageId in meta, body preserved. - const newMeta: DocmostMdMeta = { - version: meta?.version ?? 1, - ...meta, - pageId: assignedPageId, - }; - const rewritten = serializeDocmostMarkdownBody(newMeta, body); + // Write the assigned pageId back as the `gitmost_id` frontmatter, body + // preserved — the file becomes engine-tracked (SPEC §4). + const rewritten = serializePageFile(assignedPageId, body); await deps.writeFile(c.path, rewritten); writtenBack.push({ path: c.path, pageId: assignedPageId }); - // §10 loop-guard data for the created page (hash the pushed body). + // §10 loop-guard data for the created page (hash the pushed BODY). pushed.push({ pageId: assignedPageId, ...extractUpdatedAt(result), - bodyHash: bodyHash(text), + bodyHash: bodyHash(body), }); } created++; @@ -745,11 +751,11 @@ export async function applyPushActions( ); metaTable.set( `${rm.newPath}|current`, - await metaAtViaTree(deps, rm.newPath, "current"), + await metaAtViaTree(deps, rm.newPath, "current", deps.spaceId), ); metaTable.set( `${rm.oldPath}|prev`, - await metaAtViaTree(deps, rm.oldPath, "prev"), + await metaAtViaTree(deps, rm.oldPath, "prev", deps.spaceId), ); } catch (err: unknown) { prefetchFailed.add(rm.pageId); @@ -863,8 +869,58 @@ function errMessage(err: unknown): string { */ 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`; + if (slash < 0) return null; // root-level file: parent is ROOT. + const dir = path.slice(0, slash); // the enclosing folder + // The page that OWNS the enclosing folder is its folder-note `/.md`. + const folderNote = `${dir}/${baseSegment(dir)}.md`; + if (path === folderNote) { + // This path IS its folder's folder-note, so its parent is ONE LEVEL UP: the + // folder-note of the grandparent folder (or ROOT at the top level). + const up = dir.lastIndexOf("/"); + if (up < 0) return null; // top-level folder -> parent is ROOT. + const grandDir = dir.slice(0, up); + return `${grandDir}/${baseSegment(grandDir)}.md`; + } + // A leaf (or a nested folder-note) sitting inside `dir`: its parent is `dir`'s + // folder-note. + return folderNote; +} + +/** The last path segment of a forward-slash path (the folder/file base name). */ +function baseSegment(path: string): string { + const slash = path.lastIndexOf("/"); + return slash < 0 ? path : path.slice(slash + 1); +} + +/** + * The page TITLE derived from a vault path: the file's base name without the + * `.md` extension. In the native-Obsidian layout the filename IS the title — for + * a folder-note `/.md` that base equals the folder name, so the same + * rule yields the folder's title. Self-consistent across pull/push: a pulled + * (possibly disambiguated) filename round-trips to the same title, so a stable + * file never pushes a spurious rename. + */ +function titleFromPath(path: string): string { + const base = baseSegment(path); + return base.endsWith(".md") ? base.slice(0, -3) : base; +} + +/** + * Build the synthetic `DocmostMdMeta` the planner/classifier consume, from the + * NATIVE format: `pageId` from the `gitmost_id` frontmatter, `title` from the + * filename, `spaceId` from the run (the vault's space — every file belongs to + * it). `parentPageId` is intentionally absent: tree position is resolved from the + * PATH (`resolveParentPageId`), never from a stored field (SPEC §5). + */ +function nativeMeta( + text: string, + path: string, + spaceId: string, +): DocmostMdMeta { + const { id } = parsePageFile(text); + const meta: DocmostMdMeta = { version: 1, title: titleFromPath(path), spaceId }; + if (id) meta.pageId = id; + return meta; } /** @@ -899,25 +955,23 @@ async function resolveParentPageIdViaTree( 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; - } + // The parent page's identity is its `gitmost_id` frontmatter; folder position + // is irrelevant here, only the pageId. + return parsePageFile(text).id; } /** - * 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. + * Resolve the synthetic native meta at a side for the rename/move classifier (the + * title — derived from the path — comes from here). Mirrors + * `resolveParentPageIdViaTree`'s IO sides: `current` reads the working tree, + * `prev` reads `refs/docmost/last-pushed`. Returns `null` only when the file is + * missing/unreadable at that side (a real absence the classifier must see). */ async function metaAtViaTree( deps: Pick, path: string, side: MetaSide, + spaceId: string, ): Promise { let text: string | null; try { @@ -929,11 +983,7 @@ async function metaAtViaTree( return null; } if (text === null) return null; - try { - return parseDocmostMarkdown(text).meta ?? null; - } catch { - return null; - } + return nativeMeta(text, path, spaceId); } /** @@ -1156,13 +1206,13 @@ export async function runPush( if (!metaTable.has(`${currentPath}|current`)) { metaTable.set( `${currentPath}|current`, - await readMetaCurrent(deps, currentPath), + await readMetaCurrent(deps, currentPath, settings.docmostSpaceId), ); } if (!metaTable.has(`${prevPath}|prev`)) { metaTable.set( `${prevPath}|prev`, - await readMetaPrev(deps, base.ref, prevPath), + await readMetaPrev(deps, base.ref, prevPath, settings.docmostSpaceId), ); } } @@ -1179,7 +1229,8 @@ export async function runPush( if (changes.some((c) => c.status === "D")) { currentPageIds = new Set(); for (const relPath of await git.listTrackedFiles("*.md")) { - const pid = (await readMetaCurrent(deps, relPath))?.pageId; + const pid = (await readMetaCurrent(deps, relPath, settings.docmostSpaceId)) + ?.pageId; if (pid) currentPageIds.add(pid); } } @@ -1214,6 +1265,7 @@ export async function runPush( git, readFile: deps.readFile, writeFile: deps.writeFile, + spaceId: settings.docmostSpaceId, }, actions, pushedCommit, @@ -1293,10 +1345,11 @@ export async function runPush( }; } -/** Parse a file's `docmost:meta` from the live working tree (`current` side). */ +/** Synthetic native meta from the live working tree (`current` side). */ async function readMetaCurrent( deps: Pick, path: string, + spaceId: string, ): Promise { let text: string; try { @@ -1304,18 +1357,15 @@ async function readMetaCurrent( } catch { return null; // absent on disk (e.g. a D row's path) -> no current meta. } - try { - return parseDocmostMarkdown(text).meta ?? null; - } catch { - return null; // unparseable meta -> not engine-tracked. - } + return nativeMeta(text, path, spaceId); } -/** Parse a file's `docmost:meta` from the base ref's pre-image (`prev` side). */ +/** Synthetic native meta from the base ref's pre-image (`prev` side). */ async function readMetaPrev( deps: Pick, baseRef: string, path: string, + spaceId: string, ): Promise { let text: string | null; try { @@ -1324,11 +1374,7 @@ async function readMetaPrev( return null; } if (text === null) return null; // path absent at the base ref. - try { - return parseDocmostMarkdown(text).meta ?? null; - } catch { - return null; - } + return nativeMeta(text, path, spaceId); } /** Emit the full plan (counts + per-item) to the injected logger. */ diff --git a/packages/git-sync/test/apply-push-actions.test.ts b/packages/git-sync/test/apply-push-actions.test.ts index 9342f079..0a277fec 100644 --- a/packages/git-sync/test/apply-push-actions.test.ts +++ b/packages/git-sync/test/apply-push-actions.test.ts @@ -2,10 +2,11 @@ import { describe, expect, it, vi, beforeEach, afterEach } from 'vitest'; import { applyPushActions, LAST_PUSHED_REF } from '../src/engine/push'; import { bodyHash } from '../src/engine/loop-guard'; import type { ApplyPushDeps, PushActions } from '../src/engine/push'; -import { - parseDocmostMarkdown, - serializeDocmostMarkdownBody, -} from '../src/lib/index'; +import { parsePageFile, serializePageFile } from '../src/lib/page-file'; + +// The Docmost space this vault mirrors (native files carry no spaceId; the run +// supplies it). A CREATE targets this space. +const SPACE_ID = 'sp-test'; // FS→Docmost push, FIRST increment (SPEC §6). `applyPushActions` is the THIN IO // half: create/update/delete via FAKES that record every call — no real network, @@ -104,9 +105,19 @@ function deps(client: any, git: any, fs: ReturnType): ApplyPushDe git, readFile: fs.fs.readFile, writeFile: fs.fs.writeFile, + spaceId: SPACE_ID, }; } +/** + * A native page file: `gitmost_id` frontmatter + a clean body. The TITLE is NOT + * stored — it is derived from the filename — so this helper takes only a pageId. + * Used to seed both the working tree (fs) and the prev tree (showFileAtRef). + */ +function fileFor(pageId: string, body = 'body'): string { + return serializePageFile(pageId, body); +} + function actions(partial: Partial): PushActions { return { creates: [], @@ -128,9 +139,8 @@ afterEach(() => { }); describe('applyPushActions — update (collab path, SPEC §2/§15.6)', () => { - it('reads the file body and calls importPageMarkdown with it', async () => { - const fileBody = - '\n\nupdated body\n'; + it('reads the file and calls importPageMarkdown with the STRIPPED body', async () => { + const fileBody = fileFor('p-1', 'updated body'); const client = makeClient(); const { git } = makeGit(); const fs = makeFs({ 'Doc.md': fileBody }); @@ -141,9 +151,10 @@ describe('applyPushActions — update (collab path, SPEC §2/§15.6)', () => { ); expect(res.updated).toBe(1); - // The collab/Yjs write path is used — NOT a raw jsonb overwrite. + // The collab/Yjs write path is used — NOT a raw jsonb overwrite. The pushed + // content is the CLEAN body (no gitmost_id frontmatter leaks to Docmost). expect(client.importPageMarkdown).toHaveBeenCalledTimes(1); - expect(client.importPageMarkdown).toHaveBeenCalledWith('p-1', fileBody, null); + expect(client.importPageMarkdown).toHaveBeenCalledWith('p-1', 'updated body', null); // No raw-overwrite path exists on the injected client surface at all. expect((client as any).updatePageJson).toBeUndefined(); expect(client.createPage).not.toHaveBeenCalled(); @@ -151,68 +162,68 @@ describe('applyPushActions — update (collab path, SPEC §2/§15.6)', () => { }); it('forwards the last-pushed base body (3-way merge ancestor) when present', async () => { - const baseBody = - '\n\nbase body\n'; - const fileBody = - '\n\nupdated body\n'; const client = makeClient(); - // The pre-image (refs/docmost/last-pushed) carries the base version. - const { git } = makeGit({ prevTree: { 'Doc.md': baseBody } }); - const fs = makeFs({ 'Doc.md': fileBody }); + // The pre-image (refs/docmost/last-pushed) carries the base version; both + // sides are stripped to their clean body for a body-to-body 3-way merge. + const { git } = makeGit({ prevTree: { 'Doc.md': fileFor('p-1', 'base body') } }); + const fs = makeFs({ 'Doc.md': fileFor('p-1', 'updated body') }); await applyPushActions( deps(client, git, fs), actions({ updates: [{ pageId: 'p-1', path: 'Doc.md' }] }), ); - // importPageMarkdown receives the base so the server can 3-way merge it. + // importPageMarkdown receives the stripped base so the server 3-way merges it. expect(client.importPageMarkdown).toHaveBeenCalledWith( 'p-1', - fileBody, - baseBody, + 'updated body', + 'base body', ); expect(git.showFileAtRef).toHaveBeenCalledWith(LAST_PUSHED_REF, 'Doc.md'); }); }); describe('applyPushActions — create (assigned pageId written back to meta)', () => { - it('createPage is called and the new pageId is serialized back into the file', async () => { - // A brand-new local file: meta has title/spaceId but NO pageId yet. - const original = serializeDocmostMarkdownBody( - { version: 1, title: 'My New Page', spaceId: 'sp-7', parentPageId: 'parent-9' }, - '# My New Page\n\nbody text', - ); + it('createPage gets title/parent from the PATH and writes the pageId back', async () => { + // A brand-new local file with NO frontmatter (a hand-written Obsidian note) + // under a parent folder. title = filename, parent = the folder's folder-note, + // space = the run's space — all DERIVED, none stored in the file. const client = makeClient({ createId: 'page-new-42' }); const { git } = makeGit(); - const fs = makeFs({ 'New.md': original }); + const fs = makeFs({ + 'Parent/My New Page.md': '# My New Page\n\nbody text\n', + // The enclosing folder's folder-note identifies the parent page. + 'Parent/Parent.md': fileFor('parent-9'), + }); const res = await applyPushActions( deps(client, git, fs), - actions({ creates: [{ path: 'New.md' }] }), + actions({ creates: [{ path: 'Parent/My New Page.md' }] }), ); expect(res.created).toBe(1); - // createPage was called with title/body/spaceId/parentPageId from meta. expect(client.createPage).toHaveBeenCalledTimes(1); const [title, content, spaceId, parentPageId] = client.createPage.mock.calls[0]; - expect(title).toBe('My New Page'); - expect(spaceId).toBe('sp-7'); - expect(parentPageId).toBe('parent-9'); + expect(title).toBe('My New Page'); // from the filename + expect(spaceId).toBe(SPACE_ID); // from the run + expect(parentPageId).toBe('parent-9'); // from the folder's folder-note expect(content).toContain('body text'); - // The file was rewritten with the assigned pageId in meta... - expect(fs.writes.map((w) => w.path)).toEqual(['New.md']); - const rewritten = fs.store['New.md']; - const parsed = parseDocmostMarkdown(rewritten); - expect(parsed.meta?.pageId).toBe('page-new-42'); - // ...preserving the rest of the meta and the body. - expect(parsed.meta?.title).toBe('My New Page'); - expect(parsed.meta?.spaceId).toBe('sp-7'); + // The file was rewritten with the assigned pageId as gitmost_id frontmatter, + // body preserved, NO docmost:meta. + expect(fs.writes.map((w) => w.path)).toEqual(['Parent/My New Page.md']); + const rewritten = fs.store['Parent/My New Page.md']; + expect(rewritten.startsWith('---\ngitmost_id: page-new-42\n---')).toBe(true); + expect(rewritten).not.toContain('docmost:meta'); + const parsed = parsePageFile(rewritten); + expect(parsed.id).toBe('page-new-42'); expect(parsed.body).toContain('body text'); - // The write-back is recorded so a follow-up commit can be made (NEXT inc). - expect(res.writtenBack).toEqual([{ path: 'New.md', pageId: 'page-new-42' }]); + // The write-back is recorded so a follow-up commit can be made. + expect(res.writtenBack).toEqual([ + { path: 'Parent/My New Page.md', pageId: 'page-new-42' }, + ]); }); }); @@ -240,30 +251,20 @@ describe('applyPushActions — delete (soft-delete to Trash, SPEC §8)', () => { // 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. + // The new parent page owns folder `Parent/`, so its file is the FOLDER-NOTE + // `Parent/Parent.md`, whose gitmost_id is 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' }) }, + prevTree: { 'Doc.md': fileFor('p-mv') }, }); 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' }), + // Current tree: the moved file + its new parent folder's folder-note. + 'Parent/Doc.md': fileFor('p-mv'), + 'Parent/Parent.md': fileFor('parent-id'), }); const res = await applyPushActions( @@ -290,14 +291,14 @@ describe('applyPushActions — move-to-root (newParent null; SPEC §16)', () => 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). + // page whose folder-note is `Parent/Parent.md` (parent-id). prevTree: { - 'Parent/Doc.md': fileWith({ pageId: 'p-mv', title: 'Doc' }), - 'Parent.md': fileWith({ pageId: 'parent-id', title: 'Parent' }), + 'Parent/Doc.md': fileFor('p-mv'), + 'Parent/Parent.md': fileFor('parent-id'), }, }); // 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 fs = makeFs({ 'Doc.md': fileFor('p-mv') }); const res = await applyPushActions( deps(client, git, fs), @@ -315,19 +316,19 @@ describe('applyPushActions — move-to-root (newParent null; SPEC §16)', () => }); 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. + it('calls renamePage(pageId, title-from-filename) and NOT movePage', async () => { + // Same enclosing folder on both sides (parent unchanged), the FILENAME (= + // title) changed Old -> New -> a pure rename to the new filename's title. 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' }), + 'Folder/Old.md': fileFor('p-rn'), + 'Folder/Folder.md': fileFor('folder-id'), }, }); const fs = makeFs({ - 'Folder/New.md': fileWith({ pageId: 'p-rn', title: 'New Title' }), - 'Folder.md': fileWith({ pageId: 'folder-id', title: 'Folder' }), + 'Folder/New.md': fileFor('p-rn'), + 'Folder/Folder.md': fileFor('folder-id'), }); const res = await applyPushActions( @@ -342,7 +343,8 @@ describe('applyPushActions — rename (same parent, title changed; SPEC §5/§6) expect(res.renamed).toBe(1); expect(res.moved).toBe(0); expect(client.renamePage).toHaveBeenCalledTimes(1); - expect(client.renamePage).toHaveBeenCalledWith('p-rn', 'New Title'); + // The title is the NEW filename (no extension), not a stored meta title. + expect(client.renamePage).toHaveBeenCalledWith('p-rn', 'New'); expect(client.movePage).not.toHaveBeenCalled(); }); }); @@ -360,13 +362,13 @@ describe('applyPushActions — both (reparent + retitle; move THEN 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' }) }, + // Prev: at root (parent ROOT), filename `Old`. + prevTree: { 'Old.md': fileFor('p-x') }, }); 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' }), + // Current: under a new folder (folder-note np-id) AND renamed to `New`. + 'NewParent/New.md': fileFor('p-x'), + 'NewParent/NewParent.md': fileFor('np-id'), }); const res = await applyPushActions( @@ -387,41 +389,43 @@ describe('applyPushActions — both (reparent + retitle; move THEN rename)', () }); }); -describe('applyPushActions — noop (path-only rename; NO Docmost call; SPEC §5)', () => { +describe('applyPushActions — noop (parent folder renamed; 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. + // The PARENT folder was renamed Old/ -> New/ (a retitle of the parent page, + // whose folder-note kept the SAME gitmost_id). For this CHILD, neither its + // own title (`Child`) nor its parent PAGE (same id `parent-P`) changed — only + // an ancestor's name did. The page is its pageId; Docmost is 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' }), + 'Old/Child.md': fileFor('p-noop'), + 'Old/Old.md': fileFor('parent-P'), }, }); const fs = makeFs({ - 'Folder/B.md': fileWith({ pageId: 'p-noop', title: 'Same' }), - 'Folder.md': fileWith({ pageId: 'folder-id', title: 'Folder' }), + 'New/Child.md': fileFor('p-noop'), + 'New/New.md': fileFor('parent-P'), }); const res = await applyPushActions( deps(client, git, fs), actions({ renamesMoves: [ - { pageId: 'p-noop', oldPath: 'Folder/A.md', newPath: 'Folder/B.md' }, + { pageId: 'p-noop', oldPath: 'Old/Child.md', newPath: 'New/Child.md' }, ], }), ); expect(res.moved).toBe(0); expect(res.renamed).toBe(0); - // ZERO Docmost calls for a cosmetic rename. + // ZERO Docmost calls — only the ancestor folder name changed. 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', + oldPath: 'Old/Child.md', + newPath: 'New/Child.md', reason: 'path-only-rename', }, ]); @@ -435,11 +439,11 @@ describe('applyPushActions — move whose client call throws (SPEC §12 isolatio throw new Error('move boom'); }); const { git, updateRefCalls, ffCalls } = makeGit({ - prevTree: { 'Doc.md': fileWith({ pageId: 'p-mv', title: 'Doc' }) }, + prevTree: { 'Doc.md': fileFor('p-mv') }, }); const fs = makeFs({ - 'Parent/Doc.md': fileWith({ pageId: 'p-mv', title: 'Doc' }), - 'Parent.md': fileWith({ pageId: 'parent-id', title: 'Parent' }), + 'Parent/Doc.md': fileFor('p-mv'), + 'Parent/Parent.md': fileFor('parent-id'), }); const res = await applyPushActions( @@ -589,8 +593,7 @@ describe('applyPushActions — per-page error isolation + refs gated on success describe('applyPushActions — loop-guard push record (SPEC §10)', () => { it('records pageId + updatedAt + bodyHash per applied update', async () => { - const fileBody = - '\n\nupdated body\n'; + const fileBody = fileFor('p-1', 'updated body'); const client = { importPageMarkdown: vi.fn(async (_pageId: string, _md: string) => ({ // The write returns an updatedAt the loop-guard records. @@ -611,16 +614,14 @@ describe('applyPushActions — loop-guard push record (SPEC §10)', () => { expect(res.pushed).toHaveLength(1); expect(res.pushed[0].pageId).toBe('p-1'); expect(res.pushed[0].updatedAt).toBe('2026-06-20T10:00:00.000Z'); - // The bodyHash is a stable sha256 hex of the pushed markdown. - expect(res.pushed[0].bodyHash).toBe(bodyHash(fileBody)); + // The bodyHash is a stable sha256 hex of the pushed BODY (frontmatter stripped). + expect(res.pushed[0].bodyHash).toBe(bodyHash('updated body')); expect(res.pushed[0].bodyHash).toMatch(/^[0-9a-f]{64}$/); }); it('omits updatedAt when the client result does not expose one', async () => { - const newFile = serializeDocmostMarkdownBody( - { version: 1, title: 'N', spaceId: 'sp' }, - 'fresh body', - ); + // A hand-written file with no frontmatter; its body is the whole text. + const newFile = '# N\n\nfresh body\n'; const client = makeClient({ createId: 'created-9' }); const { git } = makeGit(); const fs = makeFs({ 'N.md': newFile }); @@ -633,19 +634,16 @@ describe('applyPushActions — loop-guard push record (SPEC §10)', () => { expect(res.pushed).toHaveLength(1); expect(res.pushed[0].pageId).toBe('created-9'); expect(res.pushed[0].updatedAt).toBeUndefined(); - // bodyHash of the ORIGINAL pushed file text (what createPage received). - expect(res.pushed[0].bodyHash).toBe(bodyHash(newFile)); + // bodyHash of the pushed BODY (parsePageFile strips nothing here — no + // frontmatter — so it is the trimmed file text). + expect(res.pushed[0].bodyHash).toBe(bodyHash(parsePageFile(newFile).body)); }); }); describe('applyPushActions — mixed batch + skipped passthrough', () => { it('applies update + create + delete and carries skipped rows through', async () => { - const updFile = - '\n\nupd\n'; - const newFile = serializeDocmostMarkdownBody( - { version: 1, title: 'N', spaceId: 'sp' }, - 'fresh body', - ); + const updFile = fileFor('u-1', 'upd'); + const newFile = '# N\n\nfresh body\n'; const client = makeClient({ createId: 'created-1' }); const { git, updateRefCalls } = makeGit(); const fs = makeFs({ 'U.md': updFile, 'N.md': newFile }); @@ -673,7 +671,8 @@ describe('applyPushActions — mixed batch + skipped passthrough', () => { expect(res.writtenBack).toEqual([{ path: 'N.md', pageId: 'created-1' }]); expect(res.skipped).toEqual(skipped); expect(updateRefCalls).toEqual([{ ref: LAST_PUSHED_REF, target: 'sha-9' }]); - expect(client.importPageMarkdown).toHaveBeenCalledWith('u-1', updFile, null); + // The update pushes the STRIPPED body ('upd'), not the frontmatter file. + expect(client.importPageMarkdown).toHaveBeenCalledWith('u-1', 'upd', null); expect(client.deletePage).toHaveBeenCalledWith('d-1'); }); }); diff --git a/packages/git-sync/test/engine-gaps.test.ts b/packages/git-sync/test/engine-gaps.test.ts index 61866305..f6530aec 100644 --- a/packages/git-sync/test/engine-gaps.test.ts +++ b/packages/git-sync/test/engine-gaps.test.ts @@ -8,7 +8,7 @@ import { firstDivergence } from '../src/engine/roundtrip-helpers'; import { applyPullActions } from '../src/engine/pull'; import type { PullActions, ApplyPullActionsDeps } from '../src/engine/pull'; import type { DeletionDecision } from '../src/engine/reconcile'; -import { serializeDocmostMarkdownBody } from '../src/lib/index'; +import { serializePageFile, parsePageFile } from '../src/lib/page-file'; // Engine-layer coverage gaps flagged by the PR #119 reviewers (test-strategy // report, Module 2 `src/engine`). Each block targets a specific under-covered @@ -17,13 +17,15 @@ import { serializeDocmostMarkdownBody } from '../src/lib/index'; // --- 1. push.ts:parentFolderFile — move<->rename classification lynchpin ----- // -// `parentFolderFile(path)` returns the parent FOLDER's `.md` file for a vault- -// relative path (the enclosing folder one level up, SPEC §5 path-as-truth), or -// `null` for a root-level path with no enclosing folder. It is the lynchpin of -// the move-vs-rename classifier, so it is tested directly here (it was only -// covered indirectly before): root-level, deep nesting, and — critically — -// names CONTAINING DOTS (the lastIndexOf('/') split must not be confused by a -// dot in a segment; only the LAST slash matters). +// `parentFolderFile(path)` returns the parent PAGE's file for a vault-relative +// path (SPEC §5 path-as-truth), or `null` for a root-level page. In the native- +// Obsidian FOLDER-NOTE layout the parent page that owns a folder is its folder- +// note `/.md` (NOT `.md`). For a file that IS its folder's +// folder-note, the parent is ONE LEVEL UP (the grandparent folder's note, or +// ROOT at the top). It is the lynchpin of the move-vs-rename classifier, so it +// is tested directly: root-level, a leaf in a folder, a folder-note itself, +// deep nesting, and — critically — names CONTAINING DOTS (only the LAST slash +// splits the path). describe('parentFolderFile (push.ts)', () => { it('returns null for a root-level path (no enclosing folder)', () => { expect(parentFolderFile('Child.md')).toBeNull(); @@ -31,24 +33,33 @@ describe('parentFolderFile (push.ts)', () => { expect(parentFolderFile('README.md')).toBeNull(); }); - it('returns the immediate enclosing folder file for a one-level path', () => { - expect(parentFolderFile('Space/Child.md')).toBe('Space.md'); + it('returns the enclosing folder-note for a LEAF inside a folder', () => { + // The parent page owns folder `Space/`, so its file is the folder-note + // `Space/Space.md` — NOT `Space.md`. + expect(parentFolderFile('Space/Child.md')).toBe('Space/Space.md'); }); - it('returns the DEEPEST enclosing folder file for a deeply nested path', () => { - // Only the last slash matters: the parent is the immediate folder, turned - // into its `.md` page file (NOT the space root). + it('returns the grandparent folder-note for a FOLDER-NOTE itself', () => { + // `Space/Sub/Sub.md` IS the folder-note of `Space/Sub`; its parent is the + // folder-note one level up, `Space/Space.md`. + expect(parentFolderFile('Space/Sub/Sub.md')).toBe('Space/Space.md'); + // A top-level folder-note `Space/Space.md` has the ROOT as its parent. + expect(parentFolderFile('Space/Space.md')).toBeNull(); + }); + + it('returns the DEEPEST enclosing folder-note for a deeply nested leaf', () => { + // Only the last slash matters: the parent is the immediate folder's note. expect(parentFolderFile('Space/Parent/Sub/Child.md')).toBe( - 'Space/Parent/Sub.md', + 'Space/Parent/Sub/Sub.md', ); }); it('handles names CONTAINING DOTS without splitting on the dot', () => { // A dot in a folder/file segment must not be mistaken for the path split. - // The split is purely on the LAST '/', so the `.md` is appended to the whole - // parent dir verbatim (dots and all). - expect(parentFolderFile('Space/v1.2.3/Child.md')).toBe('Space/v1.2.3.md'); - expect(parentFolderFile('a.b/c.d.md')).toBe('a.b.md'); + // The split is purely on the LAST '/', so the folder-note is the dotted + // folder name repeated inside it (dots and all). + expect(parentFolderFile('Space/v1.2.3/Child.md')).toBe('Space/v1.2.3/v1.2.3.md'); + expect(parentFolderFile('a.b/c.d.md')).toBe('a.b/a.b.md'); // A dotted root-level name still has no enclosing folder. expect(parentFolderFile('v1.2.3.md')).toBeNull(); }); @@ -271,10 +282,7 @@ describe('applyPushActions (push.ts) — move prefetch isolation', () => { // The update file exists and is readable; the move's NEW-path tree reads // throw (simulating an unreadable/missing parent folder file at `current`). const store: Record = { - 'Up.md': serializeDocmostMarkdownBody( - { version: 1, pageId: 'u1', title: 'U', spaceId: 'sp' } as any, - 'body', - ), + 'Up.md': serializePageFile('u1', 'body'), }; const deps: ApplyPushDeps = { client, @@ -284,6 +292,7 @@ describe('applyPushActions (push.ts) — move prefetch isolation', () => { throw new Error(`unreadable ${p}`); }), writeFile: vi.fn(async () => {}), + spaceId: 'sp', }; const actions: PushActions = { creates: [], @@ -302,7 +311,7 @@ describe('applyPushActions (push.ts) — move prefetch isolation', () => { expect(res.deleted).toBe(1); expect(client.importPageMarkdown).toHaveBeenCalledWith( 'u1', - store['Up.md'], + parsePageFile(store['Up.md']).body, null, ); expect(client.deletePage).toHaveBeenCalledWith('d1'); diff --git a/packages/git-sync/test/run-push.test.ts b/packages/git-sync/test/run-push.test.ts index 97bcea75..289e9899 100644 --- a/packages/git-sync/test/run-push.test.ts +++ b/packages/git-sync/test/run-push.test.ts @@ -2,7 +2,12 @@ import { describe, expect, it, vi } from 'vitest'; import { runPush, LAST_PUSHED_REF, DOCMOST_BRANCH } from '../src/engine/push'; import type { PushDeps } from '../src/engine/push'; import type { Settings } from '../src/engine/settings'; -import { serializeDocmostMarkdownBody } from '../src/lib/index'; +import { serializePageFile } from '../src/lib/page-file'; + +/** A native page file: `gitmost_id` frontmatter + clean body (title = filename). */ +function fileFor(pageId: string, body = 'body'): string { + return serializePageFile(pageId, body); +} // runPush orchestration (SPEC §6 "ФС → Docmost"), DRY-RUN BY DEFAULT. Driven by // FAKES only — no live Docmost, git, fs, or network. Asserts the SAFE-BY-DEFAULT @@ -162,8 +167,7 @@ function makeDeps( describe('runPush — dry-run is the DEFAULT (safe)', () => { it('logs a plan, builds NO client, makes ZERO Docmost calls, advances NO refs', async () => { - const file = - '\n\nedited body\n'; + const file = fileFor('p-1', 'edited body'); const { git, calls } = makeGit({ lastPushed: 'base-sha', changes: [{ status: 'M', path: 'Doc.md' }], @@ -212,10 +216,8 @@ describe('runPush — dry-run is the DEFAULT (safe)', () => { describe('runPush — --apply is the ONLY write path', () => { it('builds the client, calls applyPushActions, records created pageIds, advances last-pushed', async () => { // A brand-new local file: meta has title + spaceId but NO pageId yet. - const newFile = serializeDocmostMarkdownBody( - { version: 1, title: 'New', spaceId: 'sp-1' }, - 'fresh body', - ); + // A brand-new hand-written file with NO frontmatter (title = filename `New`). + const newFile = 'fresh body\n'; const { git, calls, setMainSha } = makeGit({ lastPushed: 'base-sha', mainSha: 'main-1', @@ -259,10 +261,8 @@ describe('runPush — --apply is the ONLY write path', () => { // MAIN push ff succeeds (ok) but the WRITE-BACK ff diverges — the write-back // branch must escalate identically to the main branch (set divergentDocmost, // log the same prominent WARNING), so main() exits 1. - const newFile = serializeDocmostMarkdownBody( - { version: 1, title: 'New', spaceId: 'sp-1' }, - 'fresh body', - ); + // A brand-new hand-written file with NO frontmatter (title = filename `New`). + const newFile = 'fresh body\n'; const { git, calls, setMainSha } = makeGit({ lastPushed: 'base-sha', mainSha: 'main-1', @@ -300,8 +300,7 @@ describe('runPush — --apply is the ONLY write path', () => { }); it('an update goes through importPageMarkdown (collab path)', async () => { - const file = - '\n\nbody\n'; + const file = fileFor('p-9', 'body'); const { git } = makeGit({ lastPushed: 'base-sha', changes: [{ status: 'M', path: 'Doc.md' }], @@ -312,7 +311,8 @@ describe('runPush — --apply is the ONLY write path', () => { const res = await runPush(deps, { dryRun: false }); - expect(client.importPageMarkdown).toHaveBeenCalledWith('p-9', file, null); + // The pushed content is the STRIPPED body (no gitmost_id frontmatter). + expect(client.importPageMarkdown).toHaveBeenCalledWith('p-9', 'body', null); expect(res.applied?.updated).toBe(1); }); }); @@ -337,8 +337,7 @@ describe('runPush — merge-in-progress aborts (SPEC §9/§12)', () => { describe('runPush — divergent docmost escalation (SPEC §5)', () => { it('sets the escalation flag and logs a WARNING, but the apply still happened', async () => { - const file = - '\n\nbody\n'; + const file = fileFor('p-1', 'body'); const { git } = makeGit({ lastPushed: 'base-sha', changes: [{ status: 'M', path: 'Doc.md' }], @@ -406,8 +405,7 @@ describe('runPush --apply — applyPushActions edge branches', () => { // One UPDATE (file carries a pageId), whose collab write throws the raw // STRING 'boom'. Every other failure test throws an Error, so the // `String(err)` fallback in errMessage (push.ts:763) is otherwise uncovered. - const file = - '\n\nbody\n'; + const file = fileFor('p-7', 'body'); const { git, calls } = makeGit({ lastPushed: 'base-sha', changes: [{ status: 'M', path: 'Doc.md' }], @@ -441,8 +439,7 @@ describe('runPush --apply — applyPushActions edge branches', () => { it('records a thrown NON-Error OBJECT via String(err) too (no implicit message)', async () => { // A thrown object literal -> String({}) === '[object Object]'. Pins down that // errMessage stringifies (not reads a .message) for non-Error throwables. - const file = - '\n\nbody\n'; + const file = fileFor('p-8', 'body'); const { git } = makeGit({ lastPushed: 'base-sha', changes: [{ status: 'M', path: 'Doc.md' }], @@ -461,22 +458,12 @@ describe('runPush --apply — applyPushActions edge branches', () => { expect(res.failures![0].error).toBe('[object Object]'); }); - it('createPage gets title="" (and parentPageId=undefined) when meta has a spaceId but NO title', async () => { - // A brand-new local file whose meta has a (truthy) spaceId — REQUIRED for the - // planner to emit a CREATE (computePushActions case "A": `else if (meta?.spaceId)`, - // push.ts:249) — but NO title and NO parentPageId. This exercises the - // `meta?.title ?? ''` fallback (push.ts:583) and `parentPageId ?? undefined` - // (push.ts:585) on the real createPage call. - // - // NOTE(review): The spec for this case asked for meta = ONLY `{version:1}` - // (no title AND no spaceId) to exercise BOTH `?? ''` fallbacks at once. That - // input is UNREACHABLE through runPush: the PURE planner (computePushActions, - // push.ts:254-262) SKIPS an added file with no usable spaceId - // (reason 'create-without-spaceId'), so it never becomes a CREATE action and - // applyPushActions' create branch never runs. A separate test below pins that - // skip. Hence `meta?.spaceId ?? ''` can never actually fall back to '' via the - // planner — only `meta?.title ?? ''` is reachable, which this test covers. - const newFile = serializeDocmostMarkdownBody({ version: 1, spaceId: 'sp-1' }, 'fresh body'); + it('createPage derives title from the FILENAME, space from the run, parent from path', async () => { + // A brand-new hand-written file at the space ROOT (no enclosing folder). In + // the native-Obsidian format nothing is stored in the file: title comes from + // the FILENAME (`New`), spaceId from the RUN (the vault's space `space-1`), + // and parentPageId from the PATH (root -> undefined). + const newFile = 'fresh body\n'; const { git } = makeGit({ lastPushed: 'base-sha', mainSha: 'main-1', @@ -493,42 +480,36 @@ describe('runPush --apply — applyPushActions edge branches', () => { expect(client.createPage).toHaveBeenCalledTimes(1); const [title, content, spaceId, parentPageId] = (client.createPage as any).mock .calls[0]; - // `meta?.title ?? ''` -> '' (no title in meta). - expect(title).toBe(''); - // The body is passed as content... - expect(content).toBe('fresh body'); - // ...and the present spaceId flows through (it is NOT replaced by ''). - expect(spaceId).toBe('sp-1'); - // `meta?.parentPageId ?? undefined` -> undefined (absent in meta). - expect(parentPageId).toBe(undefined); + expect(title).toBe('New'); // from the filename + expect(content).toBe('fresh body'); // the stripped body + expect(spaceId).toBe('space-1'); // from the run (makeSettings) + expect(parentPageId).toBe(undefined); // root path -> no parent }); - it('an added file with meta {version:1} only (no spaceId, no title) is SKIPPED, never created', async () => { - // Documents WHY the spec's "only {version:1}" create input is unreachable: - // the planner skips it (create-without-spaceId), so createPage is never called - // and `meta?.spaceId ?? ''` cannot fall back to '' via runPush. - const file = serializeDocmostMarkdownBody({ version: 1 }, 'fresh body'); - const { git, calls } = makeGit({ + it('an added file with NO frontmatter is CREATED (space from the run), never skipped', async () => { + // Native: every file in the vault belongs to the vault's space, supplied by + // the RUN — so a brand-new hand-written file (no gitmost_id) is always a + // CREATE, never skipped for a "missing spaceId" (that legacy skip is gone). + const file = 'just some text\n'; + const { git } = makeGit({ lastPushed: 'base-sha', changes: [{ status: 'A', path: 'Orphan.md' }], }); const fs = makeFs({ 'Orphan.md': file }); - const client = makeClientFake(); + const client = makeClientFake({ createId: 'orphan-id' }); const { deps } = makeDeps(git, fs, client); const res = await runPush(deps, { dryRun: false }); expect(res.planned).toEqual({ - creates: 0, + creates: 1, updates: 0, deletes: 0, renamesMoves: 0, - skipped: 1, + skipped: 0, }); - expect(client.createPage).not.toHaveBeenCalled(); - expect(res.applied?.created).toBe(0); - expect(res.applied?.skipped).toEqual([ - { path: 'Orphan.md', status: 'A', reason: 'create-without-spaceId' }, - ]); + expect(client.createPage).toHaveBeenCalledTimes(1); + expect((client.createPage as any).mock.calls[0][0]).toBe('Orphan'); // title=filename + expect(res.applied?.created).toBe(1); }); });