diff --git a/src/git.ts b/src/git.ts index 4dbed1e..8a9a594 100644 --- a/src/git.ts +++ b/src/git.ts @@ -535,6 +535,65 @@ export class VaultGit { await this.run(["update-ref", ref, target]); } + /** + * Fast-forward `branch` to `toCommit` — but ONLY if it is a TRUE fast-forward, + * i.e. the current `branch` tip is an ancestor of `toCommit` (verified via + * `git merge-base --is-ancestor `). Used to advance the + * `docmost` mirror branch after a clean push (SPEC §6 step 3 / §10): once a + * push succeeds, Docmost already contains the pushed `main` content, so the + * mirror must reflect it — otherwise the NEXT pull would diff our own write + * back and re-pull it (loop-guard). + * + * SAFETY — never force, never clobber divergent history: + * - If `branch` IS an ancestor of `toCommit`, advance it with + * `git update-ref refs/heads/ `. The `docmost` branch is + * NOT checked out during a push (push works on `main`), so updating the ref + * directly is safe and avoids any working-tree touch. + * - If `branch` is NOT an ancestor (divergent / would-be non-fast-forward), + * do NOT move it — return `{ ok: false, reason: 'not-fast-forward' }` and + * let the caller log it. We must never overwrite a `docmost` history that + * has commits the push base does not contain. + * + * Returns `{ ok: true }` when the branch was advanced (or already at + * `toCommit`, a degenerate fast-forward), `{ ok: false, reason }` otherwise. + * A missing `branch` or `toCommit` also yields `{ ok: false }` with a reason. + */ + async fastForwardBranch( + branch: string, + toCommit: string, + ): Promise<{ ok: boolean; reason?: string }> { + const branchRef = `refs/heads/${branch}`; + // Resolve both endpoints first so a missing ref is a clean refusal, not a + // confusing `merge-base` failure. + const branchSha = await this.revParse(branchRef); + if (branchSha === null) { + return { ok: false, reason: `branch ${branch} does not exist` }; + } + const targetSha = await this.revParse(toCommit); + if (targetSha === null) { + return { ok: false, reason: `target ${toCommit} does not resolve` }; + } + // Already at the target -> a no-op fast-forward (still ok). + if (branchSha === targetSha) return { ok: true }; + + // `merge-base --is-ancestor A B` exits 0 iff A is an ancestor of B. Only a + // true ancestor is a fast-forward; anything else is divergent and refused. + const ancestor = await this.runRaw([ + "merge-base", + "--is-ancestor", + branchSha, + targetSha, + ]); + if (ancestor.code !== 0) { + return { ok: false, reason: "not-fast-forward" }; + } + + // Safe to advance: the branch is not checked out during push, so a direct + // ref update avoids a checkout/working-tree touch. + await this.updateRef(branchRef, targetSha); + return { ok: true }; + } + /** * Read a file's content at a specific ref (`git show :`), or `null` * if the path does not exist there. Used by the push direction to read the diff --git a/src/loop-guard.ts b/src/loop-guard.ts new file mode 100644 index 0000000..fb66534 --- /dev/null +++ b/src/loop-guard.ts @@ -0,0 +1,29 @@ +/** + * Loop-guard primitives (SPEC §10). The sync engine must never re-pull its OWN + * write as if it were a remote edit: after a push, the next poll will see the + * page it just wrote with a fresh `updatedAt`. To suppress that, we key on two + * signals — the body HASH of what we pushed (this module) and the `updatedAt` + * returned by the write — recorded per page at push time. + * + * This module owns the PURE, deterministic body-hash. The CONSUMPTION on the + * pull side (comparing an incoming page's body hash against the last pushed hash + * to decide "this is our own write, ignore it") is a future increment — here we + * only PRODUCE the hash and the per-page push record (see `src/push.ts`). + */ +import { createHash } from "node:crypto"; + +/** + * Stable hash of a page's markdown BODY (SPEC §10 "хэш тела"). Deterministic: + * the same input string always yields the same digest, a different input a + * different one. Used to recognize our own write later (loop suppression). + * + * We hash the body STRING as-is (UTF-8) with SHA-256 and return lowercase hex. + * SPEC §10 keys on the body hash rather than file bytes; callers decide WHAT + * counts as "the body" (here it is the exact string passed in — typically the + * self-contained markdown that was pushed). No normalization is applied: the + * caller is responsible for passing a canonical/stable representation if it + * wants hash equality across cosmetic-only differences. + */ +export function bodyHash(markdownBody: string): string { + return createHash("sha256").update(markdownBody, "utf8").digest("hex"); +} diff --git a/src/push.ts b/src/push.ts index 618aa96..d4be749 100644 --- a/src/push.ts +++ b/src/push.ts @@ -21,15 +21,15 @@ * 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-increment): loop-guard (SPEC §10) — record the `updatedAt` from - * each write response + provenance trailer so the next pull does not pull our - * own write back; suppress self-writes by body hash. + * - 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 + * CLEAN push (loop-close, below). The pull-side CONSUMPTION of that record + * (suppressing a re-pull of our own write) is TODO(next-increment). * - TODO(next-increment): FS-watcher + debounce (SPEC §7.1) that commits on * `main` and triggers a push. * - TODO(next-increment): `git push` to the git remote (SPEC §6 step 1/§7.2, * pull-rebase-push with retry). - * - TODO(next-increment): fast-forward the `docmost` mirror branch after a push - * (SPEC §6 step 3) — only `refs/docmost/last-pushed` is advanced here. * - TODO(next-increment): a runnable live `main()` wired to a real Docmost. * There is deliberately NO CLI entrypoint in this file: nothing here can run * a destructive write against a real Docmost. `applyPushActions` is reached @@ -42,6 +42,7 @@ import { type DocmostMdMeta, } from "docmost-client"; import type { DiffEntry, VaultGit } from "./git.js"; +import { bodyHash } from "./loop-guard.js"; // Re-export so callers/tests can import the diff row shape from either module. export type { DiffEntry } from "./git.js"; @@ -115,10 +116,14 @@ export interface PushActionsInput { * * Classification rules: * - `A` (added): - * - current meta has NO pageId -> CREATE (a brand-new local file; the - * page does not exist in Docmost yet). * - current meta HAS a pageId -> UPDATE (a restored/copied file whose * page already exists; we push its content rather than create a dup). + * - current meta has NO pageId but HAS a non-empty spaceId -> CREATE (a + * brand-new local file; the page does not exist in Docmost yet). + * - current meta has NO pageId and NO usable spaceId -> SKIP with reason + * `create-without-spaceId`: Docmost `create_page` REQUIRES a spaceId + * (§16), and a new local file may carry only partial human meta. We + * refuse to create rather than guess a space (SPEC §8 guard spirit). * - `M` (modified): current meta has a pageId -> UPDATE content. (If a modified * file somehow lost its pageId it is skipped — there is nothing to target.) * - `D` (deleted): recover the pageId from the PRE-IMAGE meta (`metaAt(path, @@ -149,10 +154,20 @@ export function computePushActions(input: PushActionsInput): PushActions { // Added but already carries a pageId (restored/copied file): the page // exists in Docmost, so push content as an UPDATE — never a duplicate. actions.updates.push({ pageId, path: change.path }); - } else { - // Brand-new local file -> create the page, then write the assigned - // pageId back into its meta (done in `applyPushActions`). + } else if (meta?.spaceId) { + // Brand-new local file with a target space -> create the page, then + // write the assigned pageId back into its meta (in `applyPushActions`). + // `meta.spaceId` is truthy here, so empty-string is also rejected. actions.creates.push({ path: change.path }); + } else { + // A create needs a spaceId (Docmost `create_page` requires it, §16). A + // new file with partial meta and no usable spaceId is SKIPPED rather + // than created into a guessed space (SPEC §8 guard spirit). + actions.skipped.push({ + path: change.path, + status: "A", + reason: "create-without-spaceId", + }); } break; } @@ -231,6 +246,13 @@ export function computePushActions(input: PushActionsInput): PushActions { /** The marker the push direction advances after a successful push (SPEC §5/§6). */ export const LAST_PUSHED_REF = "refs/docmost/last-pushed"; +/** + * The mirror branch fast-forwarded after a clean push (SPEC §5/§6 step 3). It + * reflects "what Docmost currently contains"; advancing it to the pushed `main` + * commit closes the loop so the next pull diffs empty for the pushed pages. + */ +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 @@ -239,7 +261,9 @@ export const LAST_PUSHED_REF = "refs/docmost/last-pushed"; * - `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). - * - `git`: only `updateRef` is used here (advance `refs/docmost/last-pushed`). + * - `git`: `updateRef` (advance `refs/docmost/last-pushed`) and + * `fastForwardBranch` (advance the `docmost` mirror after a clean push, the + * loop-close — SPEC §6 step 3 / §10). */ export interface ApplyPushDeps { client: Pick< @@ -250,7 +274,7 @@ export interface ApplyPushDeps { readFile: (path: string) => Promise; /** Write a file's full text by its vault-relative path. */ writeFile: (path: string, text: string) => Promise; - git: Pick; + git: Pick; } /** A file whose meta was rewritten with a freshly-assigned pageId (post-create). */ @@ -259,6 +283,38 @@ export interface WrittenBackPage { pageId: string; } +/** + * The per-page push record consulted by a FUTURE poll-suppression (SPEC §10): a + * pulled page whose body hash + `updatedAt` match a record here is OUR OWN write + * and must not be re-pulled. PRODUCED here; CONSUMED on the pull side later. + */ +export interface PushedPageRecord { + /** The Docmost pageId that was updated/created. */ + pageId: string; + /** + * The `updatedAt` from the create/update client result, when the result + * exposed one. Absent when the (fake) client did not return it. + */ + updatedAt?: string; + /** Stable hash of the markdown BODY that was pushed (SPEC §10 "хэш тела"). */ + bodyHash: string; +} + +/** + * One page whose operation FAILED during apply (SPEC §12 resumability). The bad + * page is isolated — recorded here — and the rest of the batch still runs; the + * 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. */ + pageId?: string; + /** The vault-relative path for create/update; absent for a delete. */ + path?: string; + /** The error message captured from the thrown error. */ + error: string; +} + /** Structured outcome of `applyPushActions` (counts + write-backs + deferred). */ export interface ApplyPushResult { created: number; @@ -271,12 +327,31 @@ export interface ApplyPushResult { * not lost. */ writtenBack: WrittenBackPage[]; + /** + * Per-page push records (pageId + optional `updatedAt` + body hash) for every + * page successfully updated/created — the §10 loop-guard data a future + * poll-suppression (pull side) will consult so it does not re-pull our own + * write. Deletes are not included (no body was pushed). + */ + pushed: PushedPageRecord[]; + /** + * Pages whose operation threw — isolated and recorded, the batch continued + * (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[]; /** Diff rows the planner could not classify (carried through for logging). */ skipped: PushActions["skipped"]; - /** Whether `refs/docmost/last-pushed` was advanced (only when `pushedCommit`). */ + /** Whether `refs/docmost/last-pushed` was advanced (only on a CLEAN push). */ lastPushedAdvanced: boolean; + /** + * Result of fast-forwarding the `docmost` mirror branch after a CLEAN push + * (the loop-close, SPEC §6 step 3 / §10). `null` when no advance was attempted + * (no `pushedCommit`, or there were failures). `{ ok:false, reason }` when a + * non-fast-forward was REFUSED (divergent `docmost` history is never clobbered). + */ + docmostFastForward: { ok: boolean; reason?: string } | null; } /** @@ -296,12 +371,32 @@ export interface ApplyPushResult { * - DELETE: `client.deletePage(pageId)` — soft-delete to Trash (SPEC §8). * - RENAME/MOVE: NOT executed — returned as `deferred` (NEXT increment). * - * After applying, if a `pushedCommit` is given, advance - * `refs/docmost/last-pushed` to it (SPEC §6 step 3). Fast-forwarding the - * `docmost` branch and the loop-guard are DEFERRED (see the module TODO list). + * 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 + * `failures[]` (with its kind + pageId/path + error) and the batch CONTINUES — + * one bad page must never block the rest. Crucially, the refs are advanced ONLY + * when `failures.length === 0`: a PARTIAL push must NOT advance + * `refs/docmost/last-pushed` or the `docmost` mirror, so a re-run retries the + * whole batch cleanly (the already-applied pages are idempotent re-applies). + * + * LOOP-CLOSE (SPEC §6 step 3 / §10). After a fully-successful push, when a + * `pushedCommit` is supplied: + * - advance `refs/docmost/last-pushed` to it (what of `main` is in Docmost), AND + * - fast-forward the `docmost` mirror branch to it via + * `git.fastForwardBranch('docmost', pushedCommit)` — so the mirror reflects + * what Docmost now contains and the NEXT pull diffs EMPTY for these pages + * (it does not re-pull our own write). The ff is REFUSED (not forced) if + * `docmost` is not an ancestor of the pushed commit; the result is surfaced + * in `docmostFastForward`. On ANY failure, NEITHER ref is advanced. + * + * LOOP-GUARD DATA (SPEC §10). For every page successfully updated/created the + * result carries a `pushed` record `{ pageId, updatedAt?, bodyHash }` — the body + * hash of what was pushed plus the write's `updatedAt` (when the client returned + * one). A future pull-side poll-suppression consults this so it does not re-pull + * our own write; producing it is in scope here, consuming it is deferred. * * @param pushedCommit The `main` commit just reflected into Docmost (SHA or - * commit-ish). When omitted, the ref is NOT advanced (e.g. a dry plan). + * commit-ish). When omitted, NEITHER ref is advanced (e.g. a dry plan). */ export async function applyPushActions( deps: ApplyPushDeps, @@ -314,59 +409,105 @@ export async function applyPushActions( let updated = 0; let deleted = 0; const writtenBack: WrittenBackPage[] = []; + const pushed: PushedPageRecord[] = []; + const failures: PushFailure[] = []; // 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. for (const u of actions.updates) { - const fullMarkdown = await deps.readFile(u.path); - await client.importPageMarkdown(u.pageId, fullMarkdown); - updated++; + try { + const fullMarkdown = await deps.readFile(u.path); + const result = await client.importPageMarkdown(u.pageId, fullMarkdown); + updated++; + // §10 loop-guard data: hash the body we pushed + capture `updatedAt`. + pushed.push({ + pageId: u.pageId, + ...extractUpdatedAt(result), + bodyHash: bodyHash(fullMarkdown), + }); + } catch (err: unknown) { + failures.push({ + kind: "update", + pageId: u.pageId, + path: u.path, + error: errMessage(err), + }); + } } // 2. CREATES — create the page, then write the assigned pageId back to meta so // the file becomes tracked (SPEC §4 "записать присвоенный pageId обратно"). + // Isolated per page like updates. for (const c of actions.creates) { - 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 - // a partial meta (e.g. title/spaceId only); spaceId is required by Docmost. - const title = meta?.title ?? ""; - const spaceId = meta?.spaceId ?? ""; - const parentPageId = meta?.parentPageId ?? undefined; - const result = await client.createPage(title, body, 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 written into meta, body preserved. - const newMeta: DocmostMdMeta = { - version: meta?.version ?? 1, - ...meta, - pageId: assignedPageId, - }; - const rewritten = serializeDocmostMarkdownBody(newMeta, body); - await deps.writeFile(c.path, rewritten); - writtenBack.push({ path: c.path, pageId: assignedPageId }); + 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); + // `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); + 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). + pushed.push({ + pageId: assignedPageId, + ...extractUpdatedAt(result), + bodyHash: bodyHash(text), + }); + } + created++; + } catch (err: unknown) { + failures.push({ kind: "create", path: c.path, error: errMessage(err) }); } - created++; } - // 3. DELETES — soft-delete to Trash (SPEC §8), obratimo. + // 3. DELETES — soft-delete to Trash (SPEC §8), reversible. Isolated per page. for (const d of actions.deletes) { - await client.deletePage(d.pageId); - deleted++; + try { + await client.deletePage(d.pageId); + deleted++; + } catch (err: unknown) { + failures.push({ + kind: "delete", + pageId: d.pageId, + error: errMessage(err), + }); + } } // 4. RENAME/MOVE — DEFERRED (NEXT increment): no client call. Returned as // `deferred` so the caller can see what still needs the move/rename apply. - // 5. Advance `refs/docmost/last-pushed` to the pushed `main` commit (SPEC §6 - // step 3 / §5). TODO(next-increment): fast-forward the `docmost` mirror - // branch (Docmost already contains these changes) and record the `updatedAt` - // from each write response for the loop-guard (SPEC §10). + // 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 + // retries the whole batch (SPEC §12). The loop-close (SPEC §6 step 3 / §10): + // advance `refs/docmost/last-pushed` AND fast-forward the `docmost` mirror, + // so Docmost's new content is mirrored and the next pull diffs empty. let lastPushedAdvanced = false; - if (pushedCommit) { + let docmostFastForward: { ok: boolean; reason?: string } | null = null; + if (pushedCommit && failures.length === 0) { await git.updateRef(LAST_PUSHED_REF, pushedCommit); lastPushedAdvanced = true; + // Fast-forward the mirror (refused, not forced, on a non-fast-forward — the + // caller logs the reason). Surfaced in the result. + docmostFastForward = await git.fastForwardBranch( + DOCMOST_BRANCH, + pushedCommit, + ); } return { @@ -374,8 +515,30 @@ export async function applyPushActions( updated, deleted, writtenBack, + pushed, + failures, deferred: actions.renamesMoves, skipped: actions.skipped, lastPushedAdvanced, + docmostFastForward, }; } + +/** Stringify a thrown value into a stable error message. */ +function errMessage(err: unknown): string { + return err instanceof Error ? err.message : String(err); +} + +/** + * Pull an `updatedAt` out of a create/update client result, if present. The + * shape is `{ data: { updatedAt? }, ... }` (createPage) or a flatter object; + * absent in the simple fakes, so the field is omitted rather than `undefined`. + */ +function extractUpdatedAt(result: unknown): { updatedAt?: string } { + const r = result as + | { updatedAt?: unknown; data?: { updatedAt?: unknown } } + | null + | undefined; + const raw = r?.data?.updatedAt ?? r?.updatedAt; + return typeof raw === "string" ? { updatedAt: raw } : {}; +} diff --git a/test/apply-push-actions.test.ts b/test/apply-push-actions.test.ts index aa9536e..01e6264 100644 --- a/test/apply-push-actions.test.ts +++ b/test/apply-push-actions.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it, vi, beforeEach, afterEach } from 'vitest'; import { applyPushActions, LAST_PUSHED_REF } from '../src/push.js'; +import { bodyHash } from '../src/loop-guard.js'; import type { ApplyPushDeps, PushActions } from '../src/push.js'; import { parseDocmostMarkdown, @@ -36,15 +37,24 @@ function makeClient(opts?: { createId?: string }) { return client; } -/** A recording git fake (only updateRef is used by the push applier). */ -function makeGit() { +/** + * A recording git fake: `updateRef` (advance last-pushed) and `fastForwardBranch` + * (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 } }) { const updateRefCalls: { ref: string; target: string }[] = []; + const ffCalls: { branch: string; toCommit: string }[] = []; const git = { updateRef: vi.fn(async (ref: string, target: string) => { updateRefCalls.push({ ref, target }); }), + fastForwardBranch: vi.fn(async (branch: string, toCommit: string) => { + ffCalls.push({ branch, toCommit }); + return opts?.ffResult ?? { ok: true }; + }), }; - return { git, updateRefCalls }; + return { git, updateRefCalls, ffCalls }; } /** A recording fs fake over a path->text store. */ @@ -199,10 +209,10 @@ describe('applyPushActions — rename/move is DEFERRED (NEXT increment)', () => }); }); -describe('applyPushActions — last-pushed ref advance (SPEC §6 step 3)', () => { - it('advances refs/docmost/last-pushed to the pushed commit', async () => { +describe('applyPushActions — loop-close: ref advance + docmost ff (SPEC §6 step 3 / §10)', () => { + it('advances last-pushed AND fast-forwards the docmost mirror on a clean push', async () => { const client = makeClient(); - const { git, updateRefCalls } = makeGit(); + const { git, updateRefCalls, ffCalls } = makeGit(); const fs = makeFs(); const res = await applyPushActions( @@ -215,9 +225,34 @@ describe('applyPushActions — last-pushed ref advance (SPEC §6 step 3)', () => expect(updateRefCalls).toEqual([ { ref: LAST_PUSHED_REF, target: 'commit-sha-abc' }, ]); + // The loop-close: the docmost mirror is fast-forwarded to the pushed commit. + expect(ffCalls).toEqual([{ branch: 'docmost', toCommit: 'commit-sha-abc' }]); + expect(res.docmostFastForward).toEqual({ ok: true }); }); - it('does NOT advance the ref when no pushed commit is given', async () => { + it('surfaces a REFUSED non-fast-forward (mirror NOT clobbered)', async () => { + const client = makeClient(); + // The ff is refused because docmost is not an ancestor of the pushed commit. + const { git, updateRefCalls, ffCalls } = makeGit({ + ffResult: { ok: false, reason: 'not-fast-forward' }, + }); + const fs = makeFs(); + + const res = await applyPushActions( + deps(client, git, fs), + actions({ deletes: [{ pageId: 'p' }] }), + 'sha-div', + ); + + // last-pushed still advances (it is our own marker), but the ff result is + // surfaced so the caller can log the refusal. + expect(res.lastPushedAdvanced).toBe(true); + expect(updateRefCalls).toEqual([{ ref: LAST_PUSHED_REF, target: 'sha-div' }]); + expect(ffCalls).toEqual([{ branch: 'docmost', toCommit: 'sha-div' }]); + expect(res.docmostFastForward).toEqual({ ok: false, reason: 'not-fast-forward' }); + }); + + it('does NOT advance either ref when no pushed commit is given', async () => { const client = makeClient(); const { git, updateRefCalls } = makeGit(); const fs = makeFs(); @@ -229,7 +264,117 @@ describe('applyPushActions — last-pushed ref advance (SPEC §6 step 3)', () => expect(res.lastPushedAdvanced).toBe(false); expect(updateRefCalls).toEqual([]); + expect(res.docmostFastForward).toBeNull(); expect(git.updateRef).not.toHaveBeenCalled(); + expect(git.fastForwardBranch).not.toHaveBeenCalled(); + }); +}); + +describe('applyPushActions — per-page error isolation + refs gated on success (SPEC §12)', () => { + it('continues the batch when an update throws; records the failure; refs NOT advanced', async () => { + // A client whose 2nd importPageMarkdown call throws — the 1st and 3rd must + // still be applied, the 2nd recorded as a failure, and NO ref advanced. + let call = 0; + const client = { + importPageMarkdown: vi.fn(async (_pageId: string, _md: string) => { + call++; + if (call === 2) throw new Error('boom on page 2'); + return { success: true }; + }), + createPage: vi.fn(), + deletePage: vi.fn(), + }; + const { git, updateRefCalls, ffCalls } = makeGit(); + const fs = makeFs({ + 'A.md': 'a body', + 'B.md': 'b body', + 'C.md': 'c body', + }); + + const res = await applyPushActions( + deps(client, git, fs), + actions({ + updates: [ + { pageId: 'p-a', path: 'A.md' }, + { pageId: 'p-b', path: 'B.md' }, + { pageId: 'p-c', path: 'C.md' }, + ], + }), + 'sha-partial', + ); + + // The 1st and 3rd were applied; the 2nd threw. + expect(res.updated).toBe(2); + expect(client.importPageMarkdown).toHaveBeenCalledTimes(3); + expect(client.importPageMarkdown).toHaveBeenNthCalledWith(1, 'p-a', 'a body'); + expect(client.importPageMarkdown).toHaveBeenNthCalledWith(3, 'p-c', 'c body'); + + // The failure is recorded with kind/pageId/path/error. + expect(res.failures).toEqual([ + { kind: 'update', pageId: 'p-b', path: 'B.md', error: 'boom on page 2' }, + ]); + + // Only the successful pages carry a loop-guard push record. + expect(res.pushed.map((p) => p.pageId)).toEqual(['p-a', 'p-c']); + + // A PARTIAL push advances NEITHER ref, so a re-run retries cleanly (§12). + expect(res.lastPushedAdvanced).toBe(false); + expect(updateRefCalls).toEqual([]); + expect(ffCalls).toEqual([]); + expect(res.docmostFastForward).toBeNull(); + expect(git.updateRef).not.toHaveBeenCalled(); + expect(git.fastForwardBranch).not.toHaveBeenCalled(); + }); +}); + +describe('applyPushActions — loop-guard push record (SPEC §10)', () => { + it('records pageId + updatedAt + bodyHash per applied update', async () => { + const fileBody = + '\n\nupdated body\n'; + const client = { + importPageMarkdown: vi.fn(async (_pageId: string, _md: string) => ({ + // The write returns an updatedAt the loop-guard records. + data: { updatedAt: '2026-06-20T10:00:00.000Z' }, + success: true, + })), + createPage: vi.fn(), + deletePage: vi.fn(), + }; + const { git } = makeGit(); + const fs = makeFs({ 'Doc.md': fileBody }); + + const res = await applyPushActions( + deps(client, git, fs), + actions({ updates: [{ pageId: 'p-1', path: 'Doc.md' }] }), + ); + + 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)); + 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', + ); + const client = makeClient({ createId: 'created-9' }); + const { git } = makeGit(); + const fs = makeFs({ 'N.md': newFile }); + + const res = await applyPushActions( + deps(client, git, fs), + actions({ creates: [{ path: 'N.md' }] }), + ); + + 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)); }); }); diff --git a/test/compute-push-actions.test.ts b/test/compute-push-actions.test.ts index 9ead7db..84afa55 100644 --- a/test/compute-push-actions.test.ts +++ b/test/compute-push-actions.test.ts @@ -36,10 +36,40 @@ describe('computePushActions — A (added)', () => { expect(actions.skipped).toEqual([]); }); - it('added file with NO meta at all -> create (treated as new)', () => { + it('added file with NO meta at all -> skipped (a create needs a spaceId)', () => { + // No meta -> no spaceId -> cannot create (Docmost create_page requires it). const changes: DiffEntry[] = [{ status: 'A', path: 'Plain.md' }]; const actions = computePushActions({ changes, metaAt: metaTable({}) }); - expect(actions.creates).toEqual([{ path: 'Plain.md' }]); + expect(actions.creates).toEqual([]); + expect(actions.skipped).toEqual([ + { path: 'Plain.md', status: 'A', reason: 'create-without-spaceId' }, + ]); + }); + + it('added file with meta but NO spaceId -> skipped (create-without-spaceId)', () => { + // Partial human meta (title only, no spaceId) -> refuse to create. + const changes: DiffEntry[] = [{ status: 'A', path: 'Partial.md' }]; + const metaAt = metaTable({ + 'Partial.md|current': meta({ title: 'Partial' }), + }); + const actions = computePushActions({ changes, metaAt }); + expect(actions.creates).toEqual([]); + expect(actions.skipped).toEqual([ + { path: 'Partial.md', status: 'A', reason: 'create-without-spaceId' }, + ]); + }); + + it('added file with an EMPTY-string spaceId -> skipped (create-without-spaceId)', () => { + // An empty spaceId is not a usable target either. + const changes: DiffEntry[] = [{ status: 'A', path: 'Empty.md' }]; + const metaAt = metaTable({ + 'Empty.md|current': meta({ title: 'E', spaceId: '' }), + }); + const actions = computePushActions({ changes, metaAt }); + expect(actions.creates).toEqual([]); + expect(actions.skipped).toEqual([ + { path: 'Empty.md', status: 'A', reason: 'create-without-spaceId' }, + ]); }); it('added file WITH a pageId (restored/copied) -> update (page exists)', () => { diff --git a/test/git.test.ts b/test/git.test.ts index 7450dbe..78e56d1 100644 --- a/test/git.test.ts +++ b/test/git.test.ts @@ -622,4 +622,89 @@ describe('VaultGit (integration; temp repo)', () => { expect(preImage).toBe(meta); expect(preImage).toContain('page-123'); }); + + it('fastForwardBranch advances a true fast-forward (the loop-close, SPEC §6 step 3)', async () => { + if (!available) return; + const vault = await freshDir(); + const git = new VaultGit(vault); + await git.ensureRepo(); + + // docmost branches off main at the initial commit; main then moves ahead. + await git.ensureBranch('docmost', 'main'); + const base = await git.revParse('refs/heads/docmost'); + + await writeFile(join(vault, 'page.md'), 'pushed content\n', 'utf8'); + await git.stageAll(); + await git.commit('push page', { authorName: BOT_AUTHOR_NAME, authorEmail: BOT_AUTHOR_EMAIL }); + const mainTip = await git.revParse('HEAD'); + + // docmost is BEHIND main and an ancestor -> a true fast-forward advances it. + expect(await git.revParse('refs/heads/docmost')).toBe(base); + const res = await git.fastForwardBranch('docmost', mainTip!); + expect(res).toEqual({ ok: true }); + // The branch now points at the pushed main commit (mirror reflects Docmost). + expect(await git.revParse('refs/heads/docmost')).toBe(mainTip); + + // It does NOT touch the working tree / current branch (still on main). + expect(await git.currentBranch()).toBe('main'); + }); + + it('fastForwardBranch is a no-op (ok) when the branch is already at the target', async () => { + if (!available) return; + const vault = await freshDir(); + const git = new VaultGit(vault); + await git.ensureRepo(); + await git.ensureBranch('docmost', 'main'); + const mainTip = await git.revParse('HEAD'); + + // Already equal -> a degenerate fast-forward, still ok, branch unchanged. + const res = await git.fastForwardBranch('docmost', mainTip!); + expect(res).toEqual({ ok: true }); + expect(await git.revParse('refs/heads/docmost')).toBe(mainTip); + }); + + it('fastForwardBranch REFUSES a non-fast-forward (never clobbers divergent history)', async () => { + if (!available) return; + const vault = await freshDir(); + const git = new VaultGit(vault); + await git.ensureRepo(); + + // Make docmost diverge: it has a commit that main does NOT contain. + await git.checkout('main'); // ensure on main first + await git.ensureBranch('docmost', 'main'); + await git.checkout('docmost'); + await writeFile(join(vault, 'only-on-docmost.md'), 'mirror-only\n', 'utf8'); + await git.stageAll(); + await git.commit('docmost-only commit', { authorName: BOT_AUTHOR_NAME, authorEmail: BOT_AUTHOR_EMAIL }); + const docmostTip = await git.revParse('refs/heads/docmost'); + + // main moves ahead independently (divergent from docmost). + await git.checkout('main'); + await writeFile(join(vault, 'only-on-main.md'), 'main-only\n', 'utf8'); + await git.stageAll(); + await git.commit('main-only commit', { authorName: BOT_AUTHOR_NAME, authorEmail: BOT_AUTHOR_EMAIL }); + const mainTip = await git.revParse('HEAD'); + + // docmost is NOT an ancestor of main -> the ff is REFUSED, branch untouched. + const res = await git.fastForwardBranch('docmost', mainTip!); + expect(res).toEqual({ ok: false, reason: 'not-fast-forward' }); + expect(await git.revParse('refs/heads/docmost')).toBe(docmostTip); + }); + + it('fastForwardBranch refuses a missing branch / unresolved target with a reason', async () => { + if (!available) return; + const vault = await freshDir(); + const git = new VaultGit(vault); + await git.ensureRepo(); + const mainTip = await git.revParse('HEAD'); + + const noBranch = await git.fastForwardBranch('nope', mainTip!); + expect(noBranch.ok).toBe(false); + expect(noBranch.reason).toContain('nope'); + + await git.ensureBranch('docmost', 'main'); + const noTarget = await git.fastForwardBranch('docmost', 'deadbeefdeadbeef'); + expect(noTarget.ok).toBe(false); + expect(noTarget.reason).toContain('deadbeefdeadbeef'); + }); }); diff --git a/test/loop-guard.test.ts b/test/loop-guard.test.ts new file mode 100644 index 0000000..891d85c --- /dev/null +++ b/test/loop-guard.test.ts @@ -0,0 +1,41 @@ +import { describe, expect, it } from 'vitest'; +import { createHash } from 'node:crypto'; +import { bodyHash } from '../src/loop-guard.js'; + +// Loop-guard body hash (SPEC §10 "хэш тела"). The hash is the signal a future +// pull-side poll-suppression uses to recognize our OWN write. It MUST be +// deterministic (same input -> same hash) and discriminating (different input -> +// different hash). + +describe('bodyHash (pure, SPEC §10)', () => { + it('is deterministic — same input yields the same hash', () => { + const body = '# Title\n\nsome body with mark\n'; + expect(bodyHash(body)).toBe(bodyHash(body)); + }); + + it('differs for different input', () => { + expect(bodyHash('alpha')).not.toBe(bodyHash('beta')); + // Even a one-character difference produces a different digest. + expect(bodyHash('alpha')).not.toBe(bodyHash('alphb')); + }); + + it('returns lowercase sha256 hex (64 chars)', () => { + const h = bodyHash('hello'); + expect(h).toMatch(/^[0-9a-f]{64}$/); + // Matches an independent sha256 of the same UTF-8 bytes. + expect(h).toBe(createHash('sha256').update('hello', 'utf8').digest('hex')); + }); + + it('hashes the empty string to the well-known sha256 empty digest', () => { + expect(bodyHash('')).toBe( + 'e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855', + ); + }); + + it('is sensitive to UTF-8 content (Cyrillic body)', () => { + expect(bodyHash('Колонка')).not.toBe(bodyHash('Колонкa')); + expect(bodyHash('Колонка')).toBe( + createHash('sha256').update('Колонка', 'utf8').digest('hex'), + ); + }); +}); diff --git a/test/markdown-roundtrip.property.test.ts b/test/markdown-roundtrip.property.test.ts index a98682a..9f3b579 100644 --- a/test/markdown-roundtrip.property.test.ts +++ b/test/markdown-roundtrip.property.test.ts @@ -1,5 +1,12 @@ -import { describe, expect, it } from 'vitest'; +import { describe, expect, it, vi } from 'vitest'; import fc from 'fast-check'; + +// These property tests run real ProseMirror<->Markdown conversion × NUM_RUNS, so +// each takes ~4–5s. Inputs are DETERMINISTIC (fixed SEED below) — the only source +// of flakiness is wall-clock: under the full suite's parallel worker load they can +// exceed vitest's default 5000ms per-test timeout. Give them ample headroom so CI +// (which gates the docker build, AGENTS.md) is deterministic regardless of load. +vi.setConfig({ testTimeout: 30000 }); // Import the converter DIRECTLY from src (NOT the docmost-client barrel) so we // match the path used by the other converter unit tests. import { convertProseMirrorToMarkdown } from '../packages/docmost-client/src/lib/markdown-converter.js';